* [PATCH 01/12] Documentation: Correction in HID output_report callback description.
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
@ 2026-03-03 6:12 ` Jingyuan Liang
2026-03-11 5:10 ` Dmitry Torokhov
2026-03-03 6:12 ` [PATCH 02/12] HID: Add BUS_SPI support and define HID_SPI_DEVICE macro Jingyuan Liang
` (10 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
From: Jarrett Schultz <jaschultz@microsoft.com>
Originally output_report callback was described as must-be asynchronous,
but that is not the case in some implementations, namely i2c-hid.
Correct the documentation to say that it may be asynchronous.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
Documentation/hid/hid-transport.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/hid/hid-transport.rst b/Documentation/hid/hid-transport.rst
index 6f1692da296c..2008cf432af1 100644
--- a/Documentation/hid/hid-transport.rst
+++ b/Documentation/hid/hid-transport.rst
@@ -327,8 +327,8 @@ The available HID callbacks are:
Send raw output report via intr channel. Used by some HID device drivers
which require high throughput for outgoing requests on the intr channel. This
- must not cause SET_REPORT calls! This must be implemented as asynchronous
- output report on the intr channel!
+ must not cause SET_REPORT calls! This call might be asynchronous, so the
+ caller should not expect an immediate response!
::
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 01/12] Documentation: Correction in HID output_report callback description.
2026-03-03 6:12 ` [PATCH 01/12] Documentation: Correction in HID output_report callback description Jingyuan Liang
@ 2026-03-11 5:10 ` Dmitry Torokhov
0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Torokhov @ 2026-03-11 5:10 UTC (permalink / raw)
To: Jingyuan Liang
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, linux-doc,
linux-kernel, linux-spi, linux-trace-kernel, devicetree, hbarnor,
Jarrett Schultz, Dmitry Antipov
On Tue, Mar 03, 2026 at 06:12:53AM +0000, Jingyuan Liang wrote:
> From: Jarrett Schultz <jaschultz@microsoft.com>
>
> Originally output_report callback was described as must-be asynchronous,
> but that is not the case in some implementations, namely i2c-hid.
> Correct the documentation to say that it may be asynchronous.
>
> Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
> Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> ---
> Documentation/hid/hid-transport.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/hid/hid-transport.rst b/Documentation/hid/hid-transport.rst
> index 6f1692da296c..2008cf432af1 100644
> --- a/Documentation/hid/hid-transport.rst
> +++ b/Documentation/hid/hid-transport.rst
> @@ -327,8 +327,8 @@ The available HID callbacks are:
>
> Send raw output report via intr channel. Used by some HID device drivers
> which require high throughput for outgoing requests on the intr channel. This
> - must not cause SET_REPORT calls! This must be implemented as asynchronous
> - output report on the intr channel!
> + must not cause SET_REPORT calls! This call might be asynchronous, so the
> + caller should not expect an immediate response!
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 02/12] HID: Add BUS_SPI support and define HID_SPI_DEVICE macro
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
2026-03-03 6:12 ` [PATCH 01/12] Documentation: Correction in HID output_report callback description Jingyuan Liang
@ 2026-03-03 6:12 ` Jingyuan Liang
2026-03-11 5:11 ` Dmitry Torokhov
2026-03-03 6:12 ` [PATCH 03/12] HID: spi-hid: add transport driver skeleton for HID over SPI bus Jingyuan Liang
` (9 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
From: Jarrett Schultz <jaschultz@microsoft.com>
If connecting a hid_device with bus field indicating BUS_SPI print out
"SPI" in the debug print.
Macro sets the bus field to BUS_SPI and uses arguments to set vendor
product fields.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/hid-core.c | 3 +++
include/linux/hid.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a5b3a8ca2fcb..813c9c743ccd 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2316,6 +2316,9 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
case BUS_I2C:
bus = "I2C";
break;
+ case BUS_SPI:
+ bus = "SPI";
+ break;
case BUS_SDW:
bus = "SOUNDWIRE";
break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index dce862cafbbd..957f322a0ebd 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -786,6 +786,8 @@ struct hid_descriptor {
.bus = BUS_BLUETOOTH, .vendor = (ven), .product = (prod)
#define HID_I2C_DEVICE(ven, prod) \
.bus = BUS_I2C, .vendor = (ven), .product = (prod)
+#define HID_SPI_DEVICE(ven, prod) \
+ .bus = BUS_SPI, .vendor = (ven), .product = (prod)
#define HID_REPORT_ID(rep) \
.report_type = (rep)
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 02/12] HID: Add BUS_SPI support and define HID_SPI_DEVICE macro
2026-03-03 6:12 ` [PATCH 02/12] HID: Add BUS_SPI support and define HID_SPI_DEVICE macro Jingyuan Liang
@ 2026-03-11 5:11 ` Dmitry Torokhov
0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Torokhov @ 2026-03-11 5:11 UTC (permalink / raw)
To: Jingyuan Liang
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, linux-doc,
linux-kernel, linux-spi, linux-trace-kernel, devicetree, hbarnor,
Jarrett Schultz, Dmitry Antipov
On Tue, Mar 03, 2026 at 06:12:54AM +0000, Jingyuan Liang wrote:
> From: Jarrett Schultz <jaschultz@microsoft.com>
>
> If connecting a hid_device with bus field indicating BUS_SPI print out
> "SPI" in the debug print.
>
> Macro sets the bus field to BUS_SPI and uses arguments to set vendor
> product fields.
>
> Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
> Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> ---
> drivers/hid/hid-core.c | 3 +++
> include/linux/hid.h | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index a5b3a8ca2fcb..813c9c743ccd 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2316,6 +2316,9 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
> case BUS_I2C:
> bus = "I2C";
> break;
> + case BUS_SPI:
> + bus = "SPI";
> + break;
> case BUS_SDW:
> bus = "SOUNDWIRE";
> break;
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index dce862cafbbd..957f322a0ebd 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -786,6 +786,8 @@ struct hid_descriptor {
> .bus = BUS_BLUETOOTH, .vendor = (ven), .product = (prod)
> #define HID_I2C_DEVICE(ven, prod) \
> .bus = BUS_I2C, .vendor = (ven), .product = (prod)
> +#define HID_SPI_DEVICE(ven, prod) \
> + .bus = BUS_SPI, .vendor = (ven), .product = (prod)
>
> #define HID_REPORT_ID(rep) \
> .report_type = (rep)
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 03/12] HID: spi-hid: add transport driver skeleton for HID over SPI bus
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
2026-03-03 6:12 ` [PATCH 01/12] Documentation: Correction in HID output_report callback description Jingyuan Liang
2026-03-03 6:12 ` [PATCH 02/12] HID: Add BUS_SPI support and define HID_SPI_DEVICE macro Jingyuan Liang
@ 2026-03-03 6:12 ` Jingyuan Liang
2026-03-03 6:12 ` [PATCH 04/12] HID: spi-hid: add spi-hid driver HID layer Jingyuan Liang
` (8 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Angela Czubak, Dmitry Antipov
From: Angela Czubak <acz@semihalf.com>
Create spi-hid folder and add Kconfig and Makefile for spi-hid driver.
Add basic device structure, definitions, and probe/remove functions.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/spi-hid/Kconfig | 15 +++
drivers/hid/spi-hid/Makefile | 9 ++
drivers/hid/spi-hid/spi-hid-core.c | 212 +++++++++++++++++++++++++++++++++++++
5 files changed, 240 insertions(+)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 920a64b66b25..c6ae23bfb75d 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1434,6 +1434,8 @@ source "drivers/hid/bpf/Kconfig"
source "drivers/hid/i2c-hid/Kconfig"
+source "drivers/hid/spi-hid/Kconfig"
+
source "drivers/hid/intel-ish-hid/Kconfig"
source "drivers/hid/amd-sfh-hid/Kconfig"
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 361a7daedeb8..6b43e789b39a 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -169,6 +169,8 @@ obj-$(CONFIG_USB_KBD) += usbhid/
obj-$(CONFIG_I2C_HID_CORE) += i2c-hid/
+obj-$(CONFIG_SPI_HID_CORE) += spi-hid/
+
obj-$(CONFIG_INTEL_ISH_HID) += intel-ish-hid/
obj-$(CONFIG_AMD_SFH_HID) += amd-sfh-hid/
diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
new file mode 100644
index 000000000000..836fdefe8345
--- /dev/null
+++ b/drivers/hid/spi-hid/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Copyright (c) 2021 Microsoft Corporation
+#
+
+menuconfig SPI_HID
+ tristate "SPI HID support"
+ default y
+ depends on SPI
+
+if SPI_HID
+
+config SPI_HID_CORE
+ tristate
+endif
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
new file mode 100644
index 000000000000..92e24cddbfc2
--- /dev/null
+++ b/drivers/hid/spi-hid/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Makefile for the SPI HID input drivers
+#
+# Copyright (c) 2021 Microsoft Corporation
+#
+
+obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
+spi-hid-objs = spi-hid-core.o
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
new file mode 100644
index 000000000000..5431c60efd50
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID over SPI protocol implementation
+ *
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ *
+ * This code is partly based on "HID over I2C protocol implementation:
+ *
+ * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
+ * Copyright (c) 2012 Red Hat, Inc
+ *
+ * which in turn is partly based on "USB HID support for Linux":
+ *
+ * Copyright (c) 1999 Andreas Gal
+ * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
+ * Copyright (c) 2007-2008 Oliver Neukum
+ * Copyright (c) 2006-2010 Jiri Kosina
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/hid-over-spi.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+
+/* struct spi_hid_conf - Conf provided to the core */
+struct spi_hid_conf {
+ u32 input_report_header_address;
+ u32 input_report_body_address;
+ u32 output_report_address;
+ u8 read_opcode;
+ u8 write_opcode;
+};
+
+/**
+ * struct spihid_ops - Ops provided to the core
+ * @power_up: do sequencing to power up the device
+ * @power_down: do sequencing to power down the device
+ * @assert_reset: do sequencing to assert the reset line
+ * @deassert_reset: do sequencing to deassert the reset line
+ */
+struct spihid_ops {
+ int (*power_up)(struct spihid_ops *ops);
+ int (*power_down)(struct spihid_ops *ops);
+ int (*assert_reset)(struct spihid_ops *ops);
+ int (*deassert_reset)(struct spihid_ops *ops);
+ void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
+};
+
+/* Driver context */
+struct spi_hid {
+ struct spi_device *spi; /* spi device. */
+ struct hid_device *hid; /* pointer to corresponding HID dev. */
+
+ struct spihid_ops *ops;
+ struct spi_hid_conf *conf;
+
+ enum hidspi_power_state power_state;
+
+ u32 regulator_error_count;
+ int regulator_last_error;
+ u32 bus_error_count;
+ int bus_last_error;
+ u32 dir_count; /* device initiated reset count. */
+};
+
+static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state)
+{
+ switch (power_state) {
+ case HIDSPI_ON:
+ return "d0";
+ case HIDSPI_SLEEP:
+ return "d2";
+ case HIDSPI_OFF:
+ return "d3";
+ default:
+ return "unknown";
+ }
+}
+
+static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
+{
+ return IRQ_HANDLED;
+}
+
+static ssize_t bus_error_count_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d (%d)\n",
+ shid->bus_error_count, shid->bus_last_error);
+}
+static DEVICE_ATTR_RO(bus_error_count);
+
+static ssize_t regulator_error_count_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d (%d)\n",
+ shid->regulator_error_count,
+ shid->regulator_last_error);
+}
+static DEVICE_ATTR_RO(regulator_error_count);
+
+static ssize_t device_initiated_reset_count_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "%d\n", shid->dir_count);
+}
+static DEVICE_ATTR_RO(device_initiated_reset_count);
+
+static struct attribute *spi_hid_attrs[] = {
+ &dev_attr_bus_error_count.attr,
+ &dev_attr_regulator_error_count.attr,
+ &dev_attr_device_initiated_reset_count.attr,
+ NULL /* Terminator */
+};
+
+static const struct attribute_group spi_hid_group = {
+ .attrs = spi_hid_attrs,
+};
+
+const struct attribute_group *spi_hid_groups[] = {
+ &spi_hid_group,
+ NULL
+};
+EXPORT_SYMBOL_GPL(spi_hid_groups);
+
+int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
+ struct spi_hid_conf *conf)
+{
+ struct device *dev = &spi->dev;
+ struct spi_hid *shid;
+ int error;
+
+ if (spi->irq <= 0)
+ return dev_err_probe(dev, spi->irq ?: -EINVAL, "Missing IRQ\n");
+
+ shid = devm_kzalloc(dev, sizeof(*shid), GFP_KERNEL);
+ if (!shid)
+ return -ENOMEM;
+
+ shid->spi = spi;
+ shid->power_state = HIDSPI_ON;
+ shid->ops = ops;
+ shid->conf = conf;
+
+ spi_set_drvdata(spi, shid);
+
+ /*
+ * At the end of probe we initialize the device:
+ * 0) assert reset, bias the interrupt line
+ * 1) sleep minimal reset delay
+ * 2) request IRQ
+ * 3) power up the device
+ * 4) deassert reset (high)
+ * After this we expect an IRQ with a reset response.
+ */
+
+ shid->ops->assert_reset(shid->ops);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ error = devm_request_threaded_irq(dev, spi->irq, NULL, spi_hid_dev_irq,
+ IRQF_ONESHOT, dev_name(&spi->dev), shid);
+ if (error) {
+ dev_err(dev, "%s: unable to request threaded IRQ.", __func__);
+ return error;
+ }
+
+ error = shid->ops->power_up(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power up.", __func__);
+ return error;
+ }
+
+ shid->ops->deassert_reset(shid->ops);
+
+ dev_dbg(dev, "%s: d3 -> %s.", __func__,
+ spi_hid_power_mode_string(shid->power_state));
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(spi_hid_core_probe);
+
+void spi_hid_core_remove(struct spi_device *spi)
+{
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ int error;
+
+ shid->ops->assert_reset(shid->ops);
+ error = shid->ops->power_down(shid->ops);
+ if (error)
+ dev_err(dev, "failed to disable regulator.");
+}
+EXPORT_SYMBOL_GPL(spi_hid_core_remove);
+
+MODULE_DESCRIPTION("HID over SPI transport driver");
+MODULE_AUTHOR("Dmitry Antipov <dmanti@microsoft.com>");
+MODULE_LICENSE("GPL");
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 04/12] HID: spi-hid: add spi-hid driver HID layer
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
` (2 preceding siblings ...)
2026-03-03 6:12 ` [PATCH 03/12] HID: spi-hid: add transport driver skeleton for HID over SPI bus Jingyuan Liang
@ 2026-03-03 6:12 ` Jingyuan Liang
2026-03-03 6:12 ` [PATCH 05/12] HID: spi-hid: add HID SPI protocol implementation Jingyuan Liang
` (7 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Dmitry Antipov, Angela Czubak
Add HID low level driver callbacks to register SPI as a HID driver, and
an external touch device as a HID device.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 519 +++++++++++++++++++++++++++++++++++++
1 file changed, 519 insertions(+)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 5431c60efd50..18b035324f06 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -20,13 +20,69 @@
* Copyright (c) 2006-2010 Jiri Kosina
*/
+#include <linux/completion.h>
+#include <linux/crc32.h>
#include <linux/device.h>
+#include <linux/err.h>
#include <linux/hid.h>
#include <linux/hid-over-spi.h>
#include <linux/interrupt.h>
+#include <linux/jiffies.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/unaligned.h>
+
+#define SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST 0x00
+
+#define SPI_HID_RESP_TIMEOUT 1000
+
+/* Protocol message size constants */
+#define SPI_HID_OUTPUT_HEADER_LEN 8
+
+/* flags */
+/*
+ * ready flag indicates that the FW is ready to accept commands and
+ * requests. The FW becomes ready after sending the report descriptor.
+ */
+#define SPI_HID_READY 0
+
+/* Raw input buffer with data from the bus */
+struct spi_hid_input_buf {
+ u8 header[HIDSPI_INPUT_HEADER_SIZE];
+ u8 body[HIDSPI_INPUT_BODY_HEADER_SIZE];
+ u8 content[];
+};
+
+/* Raw output report buffer to be put on the bus */
+struct spi_hid_output_buf {
+ u8 header[SPI_HID_OUTPUT_HEADER_LEN];
+ u8 content[];
+};
+
+/* Data necessary to send an output report */
+struct spi_hid_output_report {
+ u8 report_type;
+ u16 content_length;
+ u8 content_id;
+ u8 *content;
+};
+
+/* Processed data from a device descriptor */
+struct spi_hid_device_descriptor {
+ u16 hid_version;
+ u16 report_descriptor_length;
+ u16 max_input_length;
+ u16 max_output_length;
+ u16 max_fragment_length;
+ u16 vendor_id;
+ u16 product_id;
+ u16 version_id;
+ u8 no_output_report_ack;
+};
/* struct spi_hid_conf - Conf provided to the core */
struct spi_hid_conf {
@@ -60,8 +116,26 @@ struct spi_hid {
struct spihid_ops *ops;
struct spi_hid_conf *conf;
+ struct spi_hid_device_descriptor desc; /* HID device descriptor. */
+ struct spi_hid_output_buf *output; /* Output buffer. */
+ struct spi_hid_input_buf *input; /* Input buffer. */
+ struct spi_hid_input_buf *response; /* Response buffer. */
+
+ u16 response_length;
+ u16 bufsize;
+
enum hidspi_power_state power_state;
+ u8 reset_attempts; /* The number of reset attempts. */
+
+ unsigned long flags; /* device flags. */
+
+ /* Control lock to make sure one output transaction at a time. */
+ struct mutex output_lock;
+ struct completion output_done;
+
+ u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
+
u32 regulator_error_count;
int regulator_last_error;
u32 bus_error_count;
@@ -69,6 +143,33 @@ struct spi_hid {
u32 dir_count; /* device initiated reset count. */
};
+static struct hid_ll_driver spi_hid_ll_driver;
+
+static void spi_hid_populate_output_header(u8 *buf,
+ const struct spi_hid_conf *conf,
+ const struct spi_hid_output_report *report)
+{
+ buf[0] = conf->write_opcode;
+ put_unaligned_be24(conf->output_report_address, &buf[1]);
+ buf[4] = report->report_type;
+ put_unaligned_le16(report->content_length, &buf[5]);
+ buf[7] = report->content_id;
+}
+
+static int spi_hid_output(struct spi_hid *shid, const void *buf, u16 length)
+{
+ int error;
+
+ error = spi_write(shid->spi, buf, length);
+
+ if (error) {
+ shid->bus_error_count++;
+ shid->bus_last_error = error;
+ }
+
+ return error;
+}
+
static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state)
{
switch (power_state) {
@@ -83,11 +184,416 @@ static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state
}
}
+static void spi_hid_stop_hid(struct spi_hid *shid)
+{
+ struct hid_device *hid = shid->hid;
+
+ shid->hid = NULL;
+ clear_bit(SPI_HID_READY, &shid->flags);
+
+ if (hid)
+ hid_destroy_device(hid);
+}
+
+static int spi_hid_send_output_report(struct spi_hid *shid,
+ struct spi_hid_output_report *report)
+{
+ struct spi_hid_output_buf *buf = shid->output;
+ struct device *dev = &shid->spi->dev;
+ u16 report_length;
+ u16 padded_length;
+ u8 padding;
+ int error;
+
+ guard(mutex)(&shid->output_lock);
+ if (report->content_length > shid->desc.max_output_length) {
+ dev_err(dev, "Output report too big, content_length 0x%x.",
+ report->content_length);
+ return -E2BIG;
+ }
+
+ spi_hid_populate_output_header(buf->header, shid->conf, report);
+
+ if (report->content_length)
+ memcpy(&buf->content, report->content, report->content_length);
+
+ report_length = sizeof(buf->header) + report->content_length;
+ padded_length = round_up(report_length, 4);
+ padding = padded_length - report_length;
+ memset(&buf->content[report->content_length], 0, padding);
+
+ error = spi_hid_output(shid, buf, padded_length);
+ if (error)
+ dev_err(dev, "Failed output transfer: %d.", error);
+
+ return error;
+}
+
+static int spi_hid_sync_request(struct spi_hid *shid,
+ struct spi_hid_output_report *report)
+{
+ struct device *dev = &shid->spi->dev;
+ int error;
+
+ error = spi_hid_send_output_report(shid, report);
+ if (error)
+ return error;
+
+ error = wait_for_completion_interruptible_timeout(&shid->output_done,
+ msecs_to_jiffies(SPI_HID_RESP_TIMEOUT));
+ if (error == 0) {
+ dev_err(dev, "Response timed out.");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+/*
+ * This function returns the length of the report descriptor, or a negative
+ * error code if something went wrong.
+ */
+static int spi_hid_report_descriptor_request(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = REPORT_DESCRIPTOR,
+ .content_length = 0,
+ .content_id = SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST,
+ .content = NULL,
+ };
+ int ret;
+
+ ret = spi_hid_sync_request(shid, &report);
+ if (ret) {
+ dev_err(dev,
+ "Expected report descriptor not received: %d.", ret);
+ return ret;
+ }
+
+ ret = shid->response_length;
+ if (ret != shid->desc.report_descriptor_length) {
+ ret = min_t(unsigned int, ret, shid->desc.report_descriptor_length);
+ dev_err(dev, "Received report descriptor length doesn't match device descriptor field, using min of the two: %d.",
+ ret);
+ }
+
+ return ret;
+}
+
+static int spi_hid_create_device(struct spi_hid *shid)
+{
+ struct hid_device *hid;
+ struct device *dev = &shid->spi->dev;
+ int error;
+
+ hid = hid_allocate_device();
+ error = PTR_ERR_OR_ZERO(hid);
+ if (error) {
+ dev_err(dev, "Failed to allocate hid device: %d.", error);
+ return error;
+ }
+
+ hid->driver_data = shid->spi;
+ hid->ll_driver = &spi_hid_ll_driver;
+ hid->dev.parent = &shid->spi->dev;
+ hid->bus = BUS_SPI;
+ hid->version = shid->desc.hid_version;
+ hid->vendor = shid->desc.vendor_id;
+ hid->product = shid->desc.product_id;
+
+ snprintf(hid->name, sizeof(hid->name), "spi %04X:%04X",
+ hid->vendor, hid->product);
+ strscpy(hid->phys, dev_name(&shid->spi->dev), sizeof(hid->phys));
+
+ shid->hid = hid;
+
+ error = hid_add_device(hid);
+ if (error) {
+ dev_err(dev, "Failed to add hid device: %d.", error);
+ /*
+ * We likely got here because report descriptor request timed
+ * out. Let's disconnect and destroy the hid_device structure.
+ */
+ spi_hid_stop_hid(shid);
+ return error;
+ }
+
+ return 0;
+}
+
+static int spi_hid_get_request(struct spi_hid *shid, u8 content_id)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = GET_FEATURE,
+ .content_length = 0,
+ .content_id = content_id,
+ .content = NULL,
+ };
+ int error;
+
+ error = spi_hid_sync_request(shid, &report);
+ if (error) {
+ dev_err(dev,
+ "Expected get request response not received! Error %d.",
+ error);
+ return error;
+ }
+
+ return 0;
+}
+
+static int spi_hid_set_request(struct spi_hid *shid, u8 *arg_buf, u16 arg_len,
+ u8 content_id)
+{
+ struct spi_hid_output_report report = {
+ .report_type = SET_FEATURE,
+ .content_length = arg_len,
+ .content_id = content_id,
+ .content = arg_buf,
+ };
+
+ return spi_hid_sync_request(shid, &report);
+}
+
+/* This is a placeholder. Will be implemented in the next patch. */
static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
{
return IRQ_HANDLED;
}
+static int spi_hid_alloc_buffers(struct spi_hid *shid, size_t report_size)
+{
+ struct device *dev = &shid->spi->dev;
+ int inbufsize = sizeof(shid->input->header) + sizeof(shid->input->body) + report_size;
+ int outbufsize = sizeof(shid->output->header) + report_size;
+
+ // devm_krealloc with __GFP_ZERO ensures the new memory is initialized
+ shid->output = devm_krealloc(dev, shid->output, outbufsize, GFP_KERNEL | __GFP_ZERO);
+ shid->input = devm_krealloc(dev, shid->input, inbufsize, GFP_KERNEL | __GFP_ZERO);
+ shid->response = devm_krealloc(dev, shid->response, inbufsize, GFP_KERNEL | __GFP_ZERO);
+
+ if (!shid->output || !shid->input || !shid->response)
+ return -ENOMEM;
+
+ shid->bufsize = report_size;
+
+ return 0;
+}
+
+static int spi_hid_get_report_length(struct hid_report *report)
+{
+ return ((report->size - 1) >> 3) + 1 +
+ report->device->report_enum[report->type].numbered + 2;
+}
+
+/*
+ * Traverse the supplied list of reports and find the longest
+ */
+static void spi_hid_find_max_report(struct hid_device *hid, u32 type,
+ u16 *max)
+{
+ struct hid_report *report;
+ u16 size;
+
+ /*
+ * We should not rely on wMaxInputLength, as some devices may set it to
+ * a wrong length.
+ */
+ list_for_each_entry(report, &hid->report_enum[type].report_list, list) {
+ size = spi_hid_get_report_length(report);
+ if (*max < size)
+ *max = size;
+ }
+}
+
+/* hid_ll_driver interface functions */
+
+static int spi_hid_ll_start(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ int error = 0;
+ u16 bufsize = 0;
+
+ spi_hid_find_max_report(hid, HID_INPUT_REPORT, &bufsize);
+ spi_hid_find_max_report(hid, HID_OUTPUT_REPORT, &bufsize);
+ spi_hid_find_max_report(hid, HID_FEATURE_REPORT, &bufsize);
+
+ if (bufsize < HID_MIN_BUFFER_SIZE) {
+ dev_err(&spi->dev,
+ "HID_MIN_BUFFER_SIZE > max_input_length (%d).",
+ bufsize);
+ return -EINVAL;
+ }
+
+ if (bufsize > shid->bufsize) {
+ guard(disable_irq)(&shid->spi->irq);
+
+ error = spi_hid_alloc_buffers(shid, bufsize);
+ if (error)
+ return error;
+ }
+
+ return 0;
+}
+
+static void spi_hid_ll_stop(struct hid_device *hid)
+{
+ hid->claimed = 0;
+}
+
+static int spi_hid_ll_open(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+
+ set_bit(SPI_HID_READY, &shid->flags);
+ return 0;
+}
+
+static void spi_hid_ll_close(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+
+ clear_bit(SPI_HID_READY, &shid->flags);
+ shid->reset_attempts = 0;
+}
+
+static int spi_hid_ll_power(struct hid_device *hid, int level)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ int error = 0;
+
+ guard(mutex)(&shid->output_lock);
+ if (!shid->hid)
+ error = -ENODEV;
+
+ return error;
+}
+
+static int spi_hid_ll_parse(struct hid_device *hid)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ int error, len;
+
+ len = spi_hid_report_descriptor_request(shid);
+ if (len < 0) {
+ dev_err(dev, "Report descriptor request failed, %d.", len);
+ return len;
+ }
+
+ /*
+ * FIXME: below call returning 0 doesn't mean that the report descriptor
+ * is good. We might be caching a crc32 of a corrupted r. d. or who
+ * knows what the FW sent. Need to have a feedback loop about r. d.
+ * being ok and only then cache it.
+ */
+ error = hid_parse_report(hid, (u8 *)shid->response->content, len);
+ if (error) {
+ dev_err(dev, "failed parsing report: %d.", error);
+ return error;
+ }
+ shid->report_descriptor_crc32 = crc32_le(0,
+ (unsigned char const *)shid->response->content,
+ len);
+
+ return 0;
+}
+
+static int spi_hid_ll_raw_request(struct hid_device *hid,
+ unsigned char reportnum, __u8 *buf,
+ size_t len, unsigned char rtype, int reqtype)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ int ret;
+
+ switch (reqtype) {
+ case HID_REQ_SET_REPORT:
+ if (buf[0] != reportnum) {
+ dev_err(dev, "report id mismatch.");
+ return -EINVAL;
+ }
+
+ ret = spi_hid_set_request(shid, &buf[1], len - 1,
+ reportnum);
+ if (ret) {
+ dev_err(dev, "failed to set report.");
+ return ret;
+ }
+
+ ret = len;
+ break;
+ case HID_REQ_GET_REPORT:
+ ret = spi_hid_get_request(shid, reportnum);
+ if (ret) {
+ dev_err(dev, "failed to get report.");
+ return ret;
+ }
+
+ ret = min_t(size_t, len,
+ (shid->response->body[1] | (shid->response->body[2] << 8)) + 1);
+ buf[0] = shid->response->body[3];
+ memcpy(&buf[1], &shid->response->content, ret);
+ break;
+ default:
+ dev_err(dev, "invalid request type.");
+ return -EIO;
+ }
+
+ return ret;
+}
+
+static int spi_hid_ll_output_report(struct hid_device *hid, __u8 *buf,
+ size_t len)
+{
+ struct spi_device *spi = hid->driver_data;
+ struct spi_hid *shid = spi_get_drvdata(spi);
+ struct device *dev = &spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = OUTPUT_REPORT,
+ .content_length = len - 1,
+ .content_id = buf[0],
+ .content = &buf[1],
+ };
+ int error;
+
+ if (!test_bit(SPI_HID_READY, &shid->flags)) {
+ dev_err(dev, "%s called in unready state", __func__);
+ return -ENODEV;
+ }
+
+ if (shid->desc.no_output_report_ack)
+ error = spi_hid_send_output_report(shid, &report);
+ else
+ error = spi_hid_sync_request(shid, &report);
+
+ if (error) {
+ dev_err(dev, "failed to send output report.");
+ return error;
+ }
+
+ return len;
+}
+
+static struct hid_ll_driver spi_hid_ll_driver = {
+ .start = spi_hid_ll_start,
+ .stop = spi_hid_ll_stop,
+ .open = spi_hid_ll_open,
+ .close = spi_hid_ll_close,
+ .power = spi_hid_ll_power,
+ .parse = spi_hid_ll_parse,
+ .output_report = spi_hid_ll_output_report,
+ .raw_request = spi_hid_ll_raw_request,
+};
+
static ssize_t bus_error_count_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -158,6 +664,15 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
spi_set_drvdata(spi, shid);
+ /*
+ * we need to allocate the buffer without knowing the maximum
+ * size of the reports. Let's use SZ_2K, then we do the
+ * real computation later.
+ */
+ error = spi_hid_alloc_buffers(shid, SZ_2K);
+ if (error)
+ return error;
+
/*
* At the end of probe we initialize the device:
* 0) assert reset, bias the interrupt line
@@ -190,6 +705,8 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
dev_dbg(dev, "%s: d3 -> %s.", __func__,
spi_hid_power_mode_string(shid->power_state));
+ spi_hid_create_device(shid);
+
return 0;
}
EXPORT_SYMBOL_GPL(spi_hid_core_probe);
@@ -200,6 +717,8 @@ void spi_hid_core_remove(struct spi_device *spi)
struct device *dev = &spi->dev;
int error;
+ spi_hid_stop_hid(shid);
+
shid->ops->assert_reset(shid->ops);
error = shid->ops->power_down(shid->ops);
if (error)
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 05/12] HID: spi-hid: add HID SPI protocol implementation
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
` (3 preceding siblings ...)
2026-03-03 6:12 ` [PATCH 04/12] HID: spi-hid: add spi-hid driver HID layer Jingyuan Liang
@ 2026-03-03 6:12 ` Jingyuan Liang
2026-03-03 6:12 ` [PATCH 06/12] HID: spi_hid: add spi_hid traces Jingyuan Liang
` (6 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Dmitry Antipov, Angela Czubak
This driver follows HID Over SPI Protocol Specification 1.0 available at
https://www.microsoft.com/en-us/download/details.aspx?id=103325. The
initial version of the driver does not support: 1) multi-fragment input
reports, 2) sending GET_INPUT and COMMAND output report types and
processing their respective acknowledge input reports, and 3) device
sleep power state.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 562 ++++++++++++++++++++++++++++++++++++-
1 file changed, 557 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 18b035324f06..08865d42555f 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -23,11 +23,16 @@
#include <linux/completion.h>
#include <linux/crc32.h>
#include <linux/device.h>
+#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/hid.h>
#include <linux/hid-over-spi.h>
+#include <linux/input.h>
#include <linux/interrupt.h>
+#include <linux/irq.h>
#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/slab.h>
@@ -35,12 +40,22 @@
#include <linux/string.h>
#include <linux/sysfs.h>
#include <linux/unaligned.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
+
+/* Protocol constants */
+#define SPI_HID_READ_APPROVAL_CONSTANT 0xff
+#define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
+#define SPI_HID_INPUT_HEADER_VERSION 0x03
+#define SPI_HID_SUPPORTED_VERSION 0x0300
#define SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST 0x00
-#define SPI_HID_RESP_TIMEOUT 1000
+#define SPI_HID_MAX_RESET_ATTEMPTS 3
+#define SPI_HID_RESP_TIMEOUT 1000
/* Protocol message size constants */
+#define SPI_HID_READ_APPROVAL_LEN 5
#define SPI_HID_OUTPUT_HEADER_LEN 8
/* flags */
@@ -49,6 +64,22 @@
* requests. The FW becomes ready after sending the report descriptor.
*/
#define SPI_HID_READY 0
+/*
+ * refresh_in_progress is set to true while the refresh_device worker
+ * thread is destroying and recreating the hidraw device. When this flag
+ * is set to true, the ll_close and ll_open functions will not cause
+ * power state changes.
+ */
+#define SPI_HID_REFRESH_IN_PROGRESS 1
+/*
+ * reset_pending indicates that the device is being reset. When this flag
+ * is set to true, garbage interrupts triggered during reset will be
+ * dropped and will not cause error handling.
+ */
+#define SPI_HID_RESET_PENDING 2
+#define SPI_HID_RESET_RESPONSE 3
+#define SPI_HID_CREATE_DEVICE 4
+#define SPI_HID_ERROR 5
/* Raw input buffer with data from the bus */
struct spi_hid_input_buf {
@@ -57,6 +88,22 @@ struct spi_hid_input_buf {
u8 content[];
};
+/* Processed data from input report header */
+struct spi_hid_input_header {
+ u8 version;
+ u16 report_length;
+ u8 last_fragment_flag;
+ u8 sync_const;
+};
+
+/* Processed data from an input report */
+struct spi_hid_input_report {
+ u8 report_type;
+ u16 content_length;
+ u8 content_id;
+ u8 *content;
+};
+
/* Raw output report buffer to be put on the bus */
struct spi_hid_output_buf {
u8 header[SPI_HID_OUTPUT_HEADER_LEN];
@@ -113,6 +160,9 @@ struct spi_hid {
struct spi_device *spi; /* spi device. */
struct hid_device *hid; /* pointer to corresponding HID dev. */
+ struct spi_transfer input_transfer[2]; /* Transfer buffer for read and write. */
+ struct spi_message input_message; /* used to execute a sequence of spi transfers. */
+
struct spihid_ops *ops;
struct spi_hid_conf *conf;
@@ -130,10 +180,17 @@ struct spi_hid {
unsigned long flags; /* device flags. */
+ struct work_struct reset_work;
+
/* Control lock to make sure one output transaction at a time. */
struct mutex output_lock;
+ /* Power lock to make sure one power state change at a time. */
+ struct mutex power_lock;
struct completion output_done;
+ u8 read_approval_header[SPI_HID_READ_APPROVAL_LEN];
+ u8 read_approval_body[SPI_HID_READ_APPROVAL_LEN];
+
u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
u32 regulator_error_count;
@@ -145,6 +202,66 @@ struct spi_hid {
static struct hid_ll_driver spi_hid_ll_driver;
+static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
+ u8 *header_buf, u8 *body_buf)
+{
+ header_buf[0] = conf->read_opcode;
+ put_unaligned_be24(conf->input_report_header_address, &header_buf[1]);
+ header_buf[4] = SPI_HID_READ_APPROVAL_CONSTANT;
+
+ body_buf[0] = conf->read_opcode;
+ put_unaligned_be24(conf->input_report_body_address, &body_buf[1]);
+ body_buf[4] = SPI_HID_READ_APPROVAL_CONSTANT;
+}
+
+static void spi_hid_parse_dev_desc(const struct hidspi_dev_descriptor *raw,
+ struct spi_hid_device_descriptor *desc)
+{
+ desc->hid_version = le16_to_cpu(raw->bcd_ver);
+ desc->report_descriptor_length = le16_to_cpu(raw->rep_desc_len);
+ desc->max_input_length = le16_to_cpu(raw->max_input_len);
+ desc->max_output_length = le16_to_cpu(raw->max_output_len);
+
+ /* FIXME: multi-fragment not supported, field below not used */
+ desc->max_fragment_length = le16_to_cpu(raw->max_frag_len);
+
+ desc->vendor_id = le16_to_cpu(raw->vendor_id);
+ desc->product_id = le16_to_cpu(raw->product_id);
+ desc->version_id = le16_to_cpu(raw->version_id);
+ desc->no_output_report_ack = le16_to_cpu(raw->flags) & BIT(0);
+}
+
+static void spi_hid_populate_input_header(const u8 *buf,
+ struct spi_hid_input_header *header)
+{
+ header->version = buf[0] & 0xf;
+ header->report_length = (get_unaligned_le16(&buf[1]) & 0x3fff) * 4;
+ header->last_fragment_flag = (buf[2] & 0x40) >> 6;
+ header->sync_const = buf[3];
+}
+
+static void spi_hid_populate_input_body(const u8 *buf,
+ struct input_report_body_header *body)
+{
+ body->input_report_type = buf[0];
+ body->content_len = get_unaligned_le16(&buf[1]);
+ body->content_id = buf[3];
+}
+
+static void spi_hid_input_report_prepare(struct spi_hid_input_buf *buf,
+ struct spi_hid_input_report *report)
+{
+ struct spi_hid_input_header header;
+ struct input_report_body_header body;
+
+ spi_hid_populate_input_header(buf->header, &header);
+ spi_hid_populate_input_body(buf->body, &body);
+ report->report_type = body.input_report_type;
+ report->content_length = body.content_len;
+ report->content_id = body.content_id;
+ report->content = buf->content;
+}
+
static void spi_hid_populate_output_header(u8 *buf,
const struct spi_hid_conf *conf,
const struct spi_hid_output_report *report)
@@ -156,6 +273,33 @@ static void spi_hid_populate_output_header(u8 *buf,
buf[7] = report->content_id;
}
+static int spi_hid_input_sync(struct spi_hid *shid, void *buf, u16 length,
+ bool is_header)
+{
+ int error;
+
+ shid->input_transfer[0].tx_buf = is_header ?
+ shid->read_approval_header :
+ shid->read_approval_body;
+ shid->input_transfer[0].len = SPI_HID_READ_APPROVAL_LEN;
+
+ shid->input_transfer[1].rx_buf = buf;
+ shid->input_transfer[1].len = length;
+
+ spi_message_init_with_transfers(&shid->input_message,
+ shid->input_transfer, 2);
+
+ error = spi_sync(shid->spi, &shid->input_message);
+ if (error) {
+ dev_err(&shid->spi->dev, "Error starting sync transfer: %d.", error);
+ shid->bus_error_count++;
+ shid->bus_last_error = error;
+ return error;
+ }
+
+ return 0;
+}
+
static int spi_hid_output(struct spi_hid *shid, const void *buf, u16 length)
{
int error;
@@ -195,6 +339,50 @@ static void spi_hid_stop_hid(struct spi_hid *shid)
hid_destroy_device(hid);
}
+static void spi_hid_error(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ int error;
+
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_OFF)
+ return;
+
+ if (shid->reset_attempts++ >= SPI_HID_MAX_RESET_ATTEMPTS) {
+ dev_err(dev, "unresponsive device, aborting.");
+ spi_hid_stop_hid(shid);
+ shid->ops->assert_reset(shid->ops);
+ error = shid->ops->power_down(shid->ops);
+ if (error) {
+ dev_err(dev, "failed to disable regulator.");
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ }
+ return;
+ }
+
+ clear_bit(SPI_HID_READY, &shid->flags);
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+
+ shid->ops->assert_reset(shid->ops);
+
+ shid->power_state = HIDSPI_OFF;
+
+ /*
+ * We want to cancel pending reset work as the device is being reset
+ * to recover from an error. cancel_work_sync will put us in a deadlock
+ * because this function is scheduled in 'reset_work' and we should
+ * avoid waiting for itself.
+ */
+ cancel_work(&shid->reset_work);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ shid->power_state = HIDSPI_ON;
+
+ shid->ops->deassert_reset(shid->ops);
+}
+
static int spi_hid_send_output_report(struct spi_hid *shid,
struct spi_hid_output_report *report)
{
@@ -249,6 +437,86 @@ static int spi_hid_sync_request(struct spi_hid *shid,
return 0;
}
+/*
+ * Handle the reset response from the FW by sending a request for the device
+ * descriptor.
+ */
+static void spi_hid_reset_response(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_output_report report = {
+ .report_type = DEVICE_DESCRIPTOR,
+ .content_length = 0x0,
+ .content_id = SPI_HID_OUTPUT_REPORT_CONTENT_ID_DESC_REQUEST,
+ .content = NULL,
+ };
+ int error;
+
+ if (test_bit(SPI_HID_READY, &shid->flags)) {
+ dev_err(dev, "Spontaneous FW reset!");
+ clear_bit(SPI_HID_READY, &shid->flags);
+ shid->dir_count++;
+ }
+
+ if (shid->power_state == HIDSPI_OFF)
+ return;
+
+ error = spi_hid_sync_request(shid, &report);
+ if (error) {
+ dev_WARN_ONCE(dev, true,
+ "Failed to send device descriptor request: %d.", error);
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
+ }
+}
+
+static int spi_hid_input_report_handler(struct spi_hid *shid,
+ struct spi_hid_input_buf *buf)
+{
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_input_report r;
+ int error = 0;
+
+ if (!test_bit(SPI_HID_READY, &shid->flags) ||
+ test_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags) || !shid->hid) {
+ dev_err(dev, "HID not ready");
+ return 0;
+ }
+
+ spi_hid_input_report_prepare(buf, &r);
+
+ error = hid_input_report(shid->hid, HID_INPUT_REPORT,
+ r.content - 1, r.content_length + 1, 1);
+
+ if (error == -ENODEV || error == -EBUSY) {
+ dev_err(dev, "ignoring report --> %d.", error);
+ return 0;
+ } else if (error) {
+ dev_err(dev, "Bad input report: %d.", error);
+ }
+
+ return error;
+}
+
+static void spi_hid_response_handler(struct spi_hid *shid,
+ struct input_report_body_header *body)
+{
+ shid->response_length = body->content_len;
+ /* completion_done returns 0 if there are waiters, otherwise 1 */
+ if (completion_done(&shid->output_done)) {
+ dev_err(&shid->spi->dev, "Unexpected response report.");
+ } else {
+ if (body->input_report_type == REPORT_DESCRIPTOR_RESPONSE ||
+ body->input_report_type == GET_FEATURE_RESPONSE) {
+ memcpy(shid->response->body, shid->input->body,
+ sizeof(shid->input->body));
+ memcpy(shid->response->content, shid->input->content,
+ body->content_len);
+ }
+ complete(&shid->output_done);
+ }
+}
+
/*
* This function returns the length of the report descriptor, or a negative
* error code if something went wrong.
@@ -268,6 +536,8 @@ static int spi_hid_report_descriptor_request(struct spi_hid *shid)
if (ret) {
dev_err(dev,
"Expected report descriptor not received: %d.", ret);
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
return ret;
}
@@ -322,6 +592,205 @@ static int spi_hid_create_device(struct spi_hid *shid)
return 0;
}
+static void spi_hid_refresh_device(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ u32 new_crc32 = 0;
+ int error = 0;
+
+ error = spi_hid_report_descriptor_request(shid);
+ if (error < 0) {
+ dev_err(dev,
+ "%s: failed report descriptor request: %d",
+ __func__, error);
+ return;
+ }
+ new_crc32 = crc32_le(0, (unsigned char const *)shid->response->content,
+ (size_t)error);
+
+ /* Same report descriptor, so no need to create a new hid device. */
+ if (new_crc32 == shid->report_descriptor_crc32) {
+ set_bit(SPI_HID_READY, &shid->flags);
+ return;
+ }
+
+ shid->report_descriptor_crc32 = new_crc32;
+
+ set_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags);
+
+ spi_hid_stop_hid(shid);
+
+ error = spi_hid_create_device(shid);
+ if (error) {
+ dev_err(dev, "%s: Failed to create hid device: %d.", __func__, error);
+ return;
+ }
+
+ clear_bit(SPI_HID_REFRESH_IN_PROGRESS, &shid->flags);
+}
+
+static void spi_hid_reset_work(struct work_struct *work)
+{
+ struct spi_hid *shid =
+ container_of(work, struct spi_hid, reset_work);
+ struct device *dev = &shid->spi->dev;
+ int error = 0;
+
+ if (test_and_clear_bit(SPI_HID_RESET_RESPONSE, &shid->flags)) {
+ spi_hid_reset_response(shid);
+ return;
+ }
+
+ if (test_and_clear_bit(SPI_HID_CREATE_DEVICE, &shid->flags)) {
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_OFF) {
+ dev_err(dev, "%s: Powered off, returning", __func__);
+ return;
+ }
+
+ if (!shid->hid) {
+ error = spi_hid_create_device(shid);
+ if (error) {
+ dev_err(dev, "%s: Failed to create hid device: %d.",
+ __func__, error);
+ return;
+ }
+ } else {
+ spi_hid_refresh_device(shid);
+ }
+
+ return;
+ }
+
+ if (test_and_clear_bit(SPI_HID_ERROR, &shid->flags)) {
+ spi_hid_error(shid);
+ return;
+ }
+}
+
+static int spi_hid_process_input_report(struct spi_hid *shid,
+ struct spi_hid_input_buf *buf)
+{
+ struct spi_hid_input_header header;
+ struct input_report_body_header body;
+ struct device *dev = &shid->spi->dev;
+ struct hidspi_dev_descriptor *raw;
+
+ spi_hid_populate_input_header(buf->header, &header);
+ spi_hid_populate_input_body(buf->body, &body);
+
+ if (body.content_len > header.report_length) {
+ dev_err(dev, "Bad body length %d > %d.", body.content_len,
+ header.report_length);
+ return -EPROTO;
+ }
+
+ switch (body.input_report_type) {
+ case DATA:
+ return spi_hid_input_report_handler(shid, buf);
+ case RESET_RESPONSE:
+ clear_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ set_bit(SPI_HID_RESET_RESPONSE, &shid->flags);
+ schedule_work(&shid->reset_work);
+ break;
+ case DEVICE_DESCRIPTOR_RESPONSE:
+ /* Mark the completion done to avoid timeout */
+ spi_hid_response_handler(shid, &body);
+
+ /* Reset attempts at every device descriptor fetch */
+ shid->reset_attempts = 0;
+ raw = (struct hidspi_dev_descriptor *)buf->content;
+
+ /* Validate device descriptor length before parsing */
+ if (body.content_len != HIDSPI_DEVICE_DESCRIPTOR_SIZE) {
+ dev_err(dev, "Invalid content length %d, expected %lu.",
+ body.content_len,
+ HIDSPI_DEVICE_DESCRIPTOR_SIZE);
+ return -EPROTO;
+ }
+
+ if (le16_to_cpu(raw->dev_desc_len) !=
+ HIDSPI_DEVICE_DESCRIPTOR_SIZE) {
+ dev_err(dev,
+ "Invalid wDeviceDescLength %d, expected %lu.",
+ raw->dev_desc_len,
+ HIDSPI_DEVICE_DESCRIPTOR_SIZE);
+ return -EPROTO;
+ }
+
+ spi_hid_parse_dev_desc(raw, &shid->desc);
+
+ if (shid->desc.hid_version != SPI_HID_SUPPORTED_VERSION) {
+ dev_err(dev,
+ "Unsupported device descriptor version %4x.",
+ shid->desc.hid_version);
+ return -EPROTONOSUPPORT;
+ }
+
+ set_bit(SPI_HID_CREATE_DEVICE, &shid->flags);
+ schedule_work(&shid->reset_work);
+
+ break;
+ case OUTPUT_REPORT_RESPONSE:
+ if (shid->desc.no_output_report_ack) {
+ dev_err(dev, "Unexpected output report response.");
+ break;
+ }
+ fallthrough;
+ case GET_FEATURE_RESPONSE:
+ case SET_FEATURE_RESPONSE:
+ case REPORT_DESCRIPTOR_RESPONSE:
+ spi_hid_response_handler(shid, &body);
+ break;
+ /*
+ * FIXME: sending GET_INPUT and COMMAND reports not supported, thus
+ * throw away responses to those, they should never come.
+ */
+ case GET_INPUT_REPORT_RESPONSE:
+ case COMMAND_RESPONSE:
+ dev_err(dev, "Not a supported report type: 0x%x.",
+ body.input_report_type);
+ break;
+ default:
+ dev_err(dev, "Unknown input report: 0x%x.", body.input_report_type);
+ return -EPROTO;
+ }
+
+ return 0;
+}
+
+static int spi_hid_bus_validate_header(struct spi_hid *shid,
+ struct spi_hid_input_header *header)
+{
+ struct device *dev = &shid->spi->dev;
+
+ if (header->version != SPI_HID_INPUT_HEADER_VERSION) {
+ dev_err(dev, "Unknown input report version (v 0x%x).",
+ header->version);
+ return -EINVAL;
+ }
+
+ if (shid->desc.max_input_length != 0 &&
+ header->report_length > shid->desc.max_input_length) {
+ dev_err(dev, "Input report body size %u > max expected of %u.",
+ header->report_length, shid->desc.max_input_length);
+ return -EMSGSIZE;
+ }
+
+ if (header->last_fragment_flag != 1) {
+ dev_err(dev, "Multi-fragment reports not supported.");
+ return -EOPNOTSUPP;
+ }
+
+ if (header->sync_const != SPI_HID_INPUT_HEADER_SYNC_BYTE) {
+ dev_err(dev, "Invalid input report sync constant (0x%x).",
+ header->sync_const);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int spi_hid_get_request(struct spi_hid *shid, u8 content_id)
{
struct device *dev = &shid->spi->dev;
@@ -338,6 +807,8 @@ static int spi_hid_get_request(struct spi_hid *shid, u8 content_id)
dev_err(dev,
"Expected get request response not received! Error %d.",
error);
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
return error;
}
@@ -357,9 +828,81 @@ static int spi_hid_set_request(struct spi_hid *shid, u8 *arg_buf, u16 arg_len,
return spi_hid_sync_request(shid, &report);
}
-/* This is a placeholder. Will be implemented in the next patch. */
static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
{
+ struct spi_hid *shid = _shid;
+ struct device *dev = &shid->spi->dev;
+ struct spi_hid_input_header header;
+ int error = 0;
+
+ error = spi_hid_input_sync(shid, shid->input->header,
+ sizeof(shid->input->header), true);
+ if (error) {
+ dev_err(dev, "Failed to transfer header: %d.", error);
+ goto err;
+ }
+
+ if (shid->power_state == HIDSPI_OFF) {
+ dev_warn(dev, "Device is off after header was received.");
+ goto out;
+ }
+
+ if (shid->input_message.status < 0) {
+ dev_warn(dev, "Error reading header: %d.",
+ shid->input_message.status);
+ shid->bus_error_count++;
+ shid->bus_last_error = shid->input_message.status;
+ goto err;
+ }
+
+ spi_hid_populate_input_header(shid->input->header, &header);
+
+ error = spi_hid_bus_validate_header(shid, &header);
+ if (error) {
+ if (!test_bit(SPI_HID_RESET_PENDING, &shid->flags)) {
+ dev_err(dev, "Failed to validate header: %d.", error);
+ print_hex_dump(KERN_ERR, "spi_hid: header buffer: ",
+ DUMP_PREFIX_NONE, 16, 1, shid->input->header,
+ sizeof(shid->input->header), false);
+ shid->bus_error_count++;
+ shid->bus_last_error = error;
+ goto err;
+ }
+ goto out;
+ }
+
+ error = spi_hid_input_sync(shid, shid->input->body, header.report_length,
+ false);
+ if (error) {
+ dev_err(dev, "Failed to transfer body: %d.", error);
+ goto err;
+ }
+
+ if (shid->power_state == HIDSPI_OFF) {
+ dev_warn(dev, "Device is off after body was received.");
+ goto out;
+ }
+
+ if (shid->input_message.status < 0) {
+ dev_warn(dev, "Error reading body: %d.",
+ shid->input_message.status);
+ shid->bus_error_count++;
+ shid->bus_last_error = shid->input_message.status;
+ goto err;
+ }
+
+ error = spi_hid_process_input_report(shid, shid->input);
+ if (error) {
+ dev_err(dev, "Failed to process input report: %d.", error);
+ goto err;
+ }
+
+out:
+ return IRQ_HANDLED;
+
+err:
+ set_bit(SPI_HID_ERROR, &shid->flags);
+ schedule_work(&shid->reset_work);
return IRQ_HANDLED;
}
@@ -661,11 +1204,22 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
shid->power_state = HIDSPI_ON;
shid->ops = ops;
shid->conf = conf;
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
spi_set_drvdata(spi, shid);
+ /* Using now populated conf let's pre-calculate the read approvals */
+ spi_hid_populate_read_approvals(shid->conf, shid->read_approval_header,
+ shid->read_approval_body);
+
+ mutex_init(&shid->output_lock);
+ mutex_init(&shid->power_lock);
+ init_completion(&shid->output_done);
+
+ INIT_WORK(&shid->reset_work, spi_hid_reset_work);
+
/*
- * we need to allocate the buffer without knowing the maximum
+ * We need to allocate the buffer without knowing the maximum
* size of the reports. Let's use SZ_2K, then we do the
* real computation later.
*/
@@ -705,8 +1259,6 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
dev_dbg(dev, "%s: d3 -> %s.", __func__,
spi_hid_power_mode_string(shid->power_state));
- spi_hid_create_device(shid);
-
return 0;
}
EXPORT_SYMBOL_GPL(spi_hid_core_probe);
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 06/12] HID: spi_hid: add spi_hid traces
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
` (4 preceding siblings ...)
2026-03-03 6:12 ` [PATCH 05/12] HID: spi-hid: add HID SPI protocol implementation Jingyuan Liang
@ 2026-03-03 6:12 ` Jingyuan Liang
2026-03-03 6:12 ` [PATCH 07/12] HID: spi_hid: add ACPI support for SPI over HID Jingyuan Liang
` (5 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Dmitry Antipov, Angela Czubak
Add traces for purposed of debugging spi_hid driver.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 71 -----------------
drivers/hid/spi-hid/spi-hid-core.h | 83 ++++++++++++++++++++
include/trace/events/spi_hid.h | 156 +++++++++++++++++++++++++++++++++++++
3 files changed, 239 insertions(+), 71 deletions(-)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 08865d42555f..e3273846267e 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -81,13 +81,6 @@
#define SPI_HID_CREATE_DEVICE 4
#define SPI_HID_ERROR 5
-/* Raw input buffer with data from the bus */
-struct spi_hid_input_buf {
- u8 header[HIDSPI_INPUT_HEADER_SIZE];
- u8 body[HIDSPI_INPUT_BODY_HEADER_SIZE];
- u8 content[];
-};
-
/* Processed data from input report header */
struct spi_hid_input_header {
u8 version;
@@ -104,12 +97,6 @@ struct spi_hid_input_report {
u8 *content;
};
-/* Raw output report buffer to be put on the bus */
-struct spi_hid_output_buf {
- u8 header[SPI_HID_OUTPUT_HEADER_LEN];
- u8 content[];
-};
-
/* Data necessary to send an output report */
struct spi_hid_output_report {
u8 report_type;
@@ -118,19 +105,6 @@ struct spi_hid_output_report {
u8 *content;
};
-/* Processed data from a device descriptor */
-struct spi_hid_device_descriptor {
- u16 hid_version;
- u16 report_descriptor_length;
- u16 max_input_length;
- u16 max_output_length;
- u16 max_fragment_length;
- u16 vendor_id;
- u16 product_id;
- u16 version_id;
- u8 no_output_report_ack;
-};
-
/* struct spi_hid_conf - Conf provided to the core */
struct spi_hid_conf {
u32 input_report_header_address;
@@ -155,51 +129,6 @@ struct spihid_ops {
void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
};
-/* Driver context */
-struct spi_hid {
- struct spi_device *spi; /* spi device. */
- struct hid_device *hid; /* pointer to corresponding HID dev. */
-
- struct spi_transfer input_transfer[2]; /* Transfer buffer for read and write. */
- struct spi_message input_message; /* used to execute a sequence of spi transfers. */
-
- struct spihid_ops *ops;
- struct spi_hid_conf *conf;
-
- struct spi_hid_device_descriptor desc; /* HID device descriptor. */
- struct spi_hid_output_buf *output; /* Output buffer. */
- struct spi_hid_input_buf *input; /* Input buffer. */
- struct spi_hid_input_buf *response; /* Response buffer. */
-
- u16 response_length;
- u16 bufsize;
-
- enum hidspi_power_state power_state;
-
- u8 reset_attempts; /* The number of reset attempts. */
-
- unsigned long flags; /* device flags. */
-
- struct work_struct reset_work;
-
- /* Control lock to make sure one output transaction at a time. */
- struct mutex output_lock;
- /* Power lock to make sure one power state change at a time. */
- struct mutex power_lock;
- struct completion output_done;
-
- u8 read_approval_header[SPI_HID_READ_APPROVAL_LEN];
- u8 read_approval_body[SPI_HID_READ_APPROVAL_LEN];
-
- u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
-
- u32 regulator_error_count;
- int regulator_last_error;
- u32 bus_error_count;
- int bus_last_error;
- u32 dir_count; /* device initiated reset count. */
-};
-
static struct hid_ll_driver spi_hid_ll_driver;
static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
diff --git a/drivers/hid/spi-hid/spi-hid-core.h b/drivers/hid/spi-hid/spi-hid-core.h
new file mode 100644
index 000000000000..2bfdfbe6d7fc
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-core.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ */
+
+#include <linux/hid-over-spi.h>
+#include <linux/spi/spi.h>
+
+/* Protocol message size constants */
+#define SPI_HID_READ_APPROVAL_LEN 5
+#define SPI_HID_OUTPUT_HEADER_LEN 8
+
+/* Raw input buffer with data from the bus */
+struct spi_hid_input_buf {
+ u8 header[HIDSPI_INPUT_HEADER_SIZE];
+ u8 body[HIDSPI_INPUT_BODY_HEADER_SIZE];
+ u8 content[];
+};
+
+/* Raw output report buffer to be put on the bus */
+struct spi_hid_output_buf {
+ u8 header[SPI_HID_OUTPUT_HEADER_LEN];
+ u8 content[];
+};
+
+/* Processed data from a device descriptor */
+struct spi_hid_device_descriptor {
+ u16 hid_version;
+ u16 report_descriptor_length;
+ u16 max_input_length;
+ u16 max_output_length;
+ u16 max_fragment_length;
+ u16 vendor_id;
+ u16 product_id;
+ u16 version_id;
+ u8 no_output_report_ack;
+};
+
+/* Driver context */
+struct spi_hid {
+ struct spi_device *spi; /* spi device. */
+ struct hid_device *hid; /* pointer to corresponding HID dev. */
+
+ struct spi_transfer input_transfer[2]; /* Transfer buffer for read and write. */
+ struct spi_message input_message; /* used to execute a sequence of spi transfers. */
+
+ struct spihid_ops *ops;
+ struct spi_hid_conf *conf;
+
+ struct spi_hid_device_descriptor desc; /* HID device descriptor. */
+ struct spi_hid_output_buf *output; /* Output buffer. */
+ struct spi_hid_input_buf *input; /* Input buffer. */
+ struct spi_hid_input_buf *response; /* Response buffer. */
+
+ u16 response_length;
+ u16 bufsize;
+
+ enum hidspi_power_state power_state;
+
+ u8 reset_attempts; /* The number of reset attempts. */
+
+ unsigned long flags; /* device flags. */
+
+ struct work_struct reset_work;
+
+ /* Control lock to make sure one output transaction at a time. */
+ struct mutex output_lock;
+ /* Power lock to make sure one power state change at a time. */
+ struct mutex power_lock;
+ struct completion output_done;
+
+ u8 read_approval_header[SPI_HID_READ_APPROVAL_LEN];
+ u8 read_approval_body[SPI_HID_READ_APPROVAL_LEN];
+
+ u32 report_descriptor_crc32; /* HID report descriptor crc32 checksum. */
+
+ u32 regulator_error_count;
+ int regulator_last_error;
+ u32 bus_error_count;
+ int bus_last_error;
+ u32 dir_count; /* device initiated reset count. */
+};
diff --git a/include/trace/events/spi_hid.h b/include/trace/events/spi_hid.h
new file mode 100644
index 000000000000..e9a579b3806c
--- /dev/null
+++ b/include/trace/events/spi_hid.h
@@ -0,0 +1,156 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Microsoft Corporation
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM spi_hid
+
+#if !defined(_SPI_HID_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _SPI_HID_TRACE_H
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(spi_hid_transfer,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret),
+
+ TP_STRUCT__entry(
+ __field(int, bus_num)
+ __field(int, chip_select)
+ __field(int, ret)
+ __dynamic_array(u8, rx_buf, rx_len)
+ __dynamic_array(u8, tx_buf, tx_len)
+ ),
+
+ TP_fast_assign(
+ __entry->bus_num = shid->spi->controller->bus_num;
+ __entry->chip_select = shid->spi->chip_select;
+ __entry->ret = ret;
+
+ memcpy(__get_dynamic_array(tx_buf), tx_buf, tx_len);
+ memcpy(__get_dynamic_array(rx_buf), rx_buf, rx_len);
+ ),
+
+ TP_printk("spi%d.%d: len=%d tx=[%*phD] rx=[%*phD] --> %d",
+ __entry->bus_num, __entry->chip_select,
+ __get_dynamic_array_len(tx_buf) + __get_dynamic_array_len(rx_buf),
+ __get_dynamic_array_len(tx_buf), __get_dynamic_array(tx_buf),
+ __get_dynamic_array_len(rx_buf), __get_dynamic_array(rx_buf),
+ __entry->ret)
+);
+
+DEFINE_EVENT(spi_hid_transfer, spi_hid_input_sync,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret));
+
+DEFINE_EVENT(spi_hid_transfer, spi_hid_input_header_complete,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret));
+
+DEFINE_EVENT(spi_hid_transfer, spi_hid_input_body_complete,
+ TP_PROTO(struct spi_hid *shid, const void *tx_buf, int tx_len,
+ const void *rx_buf, u16 rx_len, int ret),
+ TP_ARGS(shid, tx_buf, tx_len, rx_buf, rx_len, ret));
+
+DECLARE_EVENT_CLASS(spi_hid_irq,
+ TP_PROTO(struct spi_hid *shid, int irq),
+
+ TP_ARGS(shid, irq),
+
+ TP_STRUCT__entry(
+ __field(int, bus_num)
+ __field(int, chip_select)
+ __field(int, irq)
+ ),
+
+ TP_fast_assign(
+ __entry->bus_num = shid->spi->controller->bus_num;
+ __entry->chip_select = shid->spi->chip_select;
+ __entry->irq = irq;
+ ),
+
+ TP_printk("spi%d.%d: IRQ %d",
+ __entry->bus_num, __entry->chip_select, __entry->irq)
+);
+
+DEFINE_EVENT(spi_hid_irq, spi_hid_dev_irq,
+ TP_PROTO(struct spi_hid *shid, int irq), TP_ARGS(shid, irq));
+
+DECLARE_EVENT_CLASS(spi_hid,
+ TP_PROTO(struct spi_hid *shid),
+
+ TP_ARGS(shid),
+
+ TP_STRUCT__entry(
+ __field(int, bus_num)
+ __field(int, chip_select)
+ __field(int, power_state)
+ __field(u32, flags)
+
+ __field(int, vendor_id)
+ __field(int, product_id)
+ __field(int, max_input_length)
+ __field(int, max_output_length)
+ __field(u16, hid_version)
+ __field(u16, report_descriptor_length)
+ __field(u16, version_id)
+ ),
+
+ TP_fast_assign(
+ __entry->bus_num = shid->spi->controller->bus_num;
+ __entry->chip_select = shid->spi->chip_select;
+ __entry->power_state = shid->power_state;
+ __entry->flags = shid->flags;
+
+ __entry->vendor_id = shid->desc.vendor_id;
+ __entry->product_id = shid->desc.product_id;
+ __entry->max_input_length = shid->desc.max_input_length;
+ __entry->max_output_length = shid->desc.max_output_length;
+ __entry->hid_version = shid->desc.hid_version;
+ __entry->report_descriptor_length =
+ shid->desc.report_descriptor_length;
+ __entry->version_id = shid->desc.version_id;
+ ),
+
+ TP_printk("spi%d.%d: (%04x:%04x v%d) HID v%d.%d state p:%d len i:%d o:%d r:%d flags 0x%08x",
+ __entry->bus_num, __entry->chip_select,
+ __entry->vendor_id, __entry->product_id, __entry->version_id,
+ __entry->hid_version >> 8, __entry->hid_version & 0xff,
+ __entry->power_state, __entry->max_input_length,
+ __entry->max_output_length, __entry->report_descriptor_length,
+ __entry->flags)
+);
+
+DEFINE_EVENT(spi_hid, spi_hid_header_transfer, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_process_input_report,
+ TP_PROTO(struct spi_hid *shid), TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_input_report_handler,
+ TP_PROTO(struct spi_hid *shid), TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_reset_response, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_create_device, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_refresh_device, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_response_handler, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+DEFINE_EVENT(spi_hid, spi_hid_error_handler, TP_PROTO(struct spi_hid *shid),
+ TP_ARGS(shid));
+
+#endif /* _SPI_HID_TRACE_H */
+
+#include <trace/define_trace.h>
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 07/12] HID: spi_hid: add ACPI support for SPI over HID
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
` (5 preceding siblings ...)
2026-03-03 6:12 ` [PATCH 06/12] HID: spi_hid: add spi_hid traces Jingyuan Liang
@ 2026-03-03 6:12 ` Jingyuan Liang
2026-03-11 5:27 ` Dmitry Torokhov
2026-03-03 6:13 ` [PATCH 08/12] HID: spi_hid: add device tree " Jingyuan Liang
` (4 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:12 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Angela Czubak
From: Angela Czubak <acz@semihalf.com>
Detect SPI HID devices described in ACPI.
Signed-off-by: Angela Czubak <acz@semihalf.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/Kconfig | 15 +++
drivers/hid/spi-hid/Makefile | 1 +
drivers/hid/spi-hid/spi-hid-acpi.c | 253 +++++++++++++++++++++++++++++++++++++
drivers/hid/spi-hid/spi-hid-core.c | 27 +---
drivers/hid/spi-hid/spi-hid.h | 44 +++++++
5 files changed, 316 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
index 836fdefe8345..114b1e00da39 100644
--- a/drivers/hid/spi-hid/Kconfig
+++ b/drivers/hid/spi-hid/Kconfig
@@ -10,6 +10,21 @@ menuconfig SPI_HID
if SPI_HID
+config SPI_HID_ACPI
+ tristate "HID over SPI transport layer ACPI driver"
+ depends on ACPI
+ select SPI_HID_CORE
+ help
+ Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
+ other HID based devices which are connected to your computer via SPI.
+ This driver supports ACPI-based systems.
+
+ If unsure, say N.
+
+ This support is also available as a module. If so, the module
+ will be called spi-hid-acpi. It will also build/depend on the
+ module spi-hid.
+
config SPI_HID_CORE
tristate
endif
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
index 92e24cddbfc2..753c7b7a7844 100644
--- a/drivers/hid/spi-hid/Makefile
+++ b/drivers/hid/spi-hid/Makefile
@@ -7,3 +7,4 @@
obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
spi-hid-objs = spi-hid-core.o
+obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
new file mode 100644
index 000000000000..612e74fe72f9
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-acpi.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID over SPI protocol, ACPI related code
+ *
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ *
+ * This code was forked out of the HID over SPI core code, which is partially
+ * based on "HID over I2C protocol implementation:
+ *
+ * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
+ * Copyright (c) 2012 Red Hat, Inc
+ *
+ * which in turn is partially based on "USB HID support for Linux":
+ *
+ * Copyright (c) 1999 Andreas Gal
+ * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
+ * Copyright (c) 2007-2008 Oliver Neukum
+ * Copyright (c) 2006-2010 Jiri Kosina
+ */
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/reset.h>
+#include <linux/uuid.h>
+
+#include "spi-hid.h"
+
+/* Config structure is filled with data from ACPI */
+struct spi_hid_acpi_config {
+ struct spihid_ops ops;
+
+ struct spi_hid_conf property_conf;
+ u32 post_power_on_delay_ms;
+ u32 minimal_reset_delay_ms;
+ struct acpi_device *adev;
+};
+
+/* HID SPI Device: 6e2ac436-0fcf41af-a265-b32a220dcfab */
+static guid_t spi_hid_guid =
+ GUID_INIT(0x6E2AC436, 0x0FCF, 0x41AF,
+ 0xA2, 0x65, 0xB3, 0x2A, 0x22, 0x0D, 0xCF, 0xAB);
+
+static int spi_hid_acpi_populate_config(struct spi_hid_acpi_config *conf,
+ struct acpi_device *adev)
+{
+ acpi_handle handle = acpi_device_handle(adev);
+ union acpi_object *obj;
+
+ conf->adev = adev;
+
+ /* Revision 3 for HID over SPI V1, see specification. */
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 1, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID input report header address failed");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_header_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 2, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID input report body address failed");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_body_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 3, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID output report header address failed");
+ return -ENODEV;
+ }
+ conf->property_conf.output_report_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 4, NULL,
+ ACPI_TYPE_BUFFER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID read opcode failed");
+ return -ENODEV;
+ }
+ if (obj->buffer.length == 1) {
+ conf->property_conf.read_opcode = obj->buffer.pointer[0];
+ } else {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID read opcode, too long buffer");
+ ACPI_FREE(obj);
+ return -ENODEV;
+ }
+ ACPI_FREE(obj);
+
+ obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 5, NULL,
+ ACPI_TYPE_BUFFER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID write opcode failed");
+ return -ENODEV;
+ }
+ if (obj->buffer.length == 1) {
+ conf->property_conf.write_opcode = obj->buffer.pointer[0];
+ } else {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID write opcode, too long buffer");
+ ACPI_FREE(obj);
+ return -ENODEV;
+ }
+ ACPI_FREE(obj);
+
+ /* Value not provided in ACPI,*/
+ conf->post_power_on_delay_ms = 5;
+ conf->minimal_reset_delay_ms = 150;
+
+ if (!acpi_has_method(handle, "_RST")) {
+ acpi_handle_err(handle, "No reset method for acpi handle");
+ return -ENODEV;
+ }
+
+ /* FIXME: not reading hid-over-spi-flags, multi-SPI not supported */
+
+ return 0;
+}
+
+static int spi_hid_acpi_power_none(struct spihid_ops *ops)
+{
+ return 0;
+}
+
+static int spi_hid_acpi_power_down(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+
+ return acpi_device_set_power(conf->adev, ACPI_STATE_D3);
+}
+
+static int spi_hid_acpi_power_up(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+ int error;
+
+ error = acpi_device_set_power(conf->adev, ACPI_STATE_D0);
+ if (error) {
+ dev_err(&conf->adev->dev, "Error could not power up ACPI device: %d.", error);
+ return error;
+ }
+
+ if (conf->post_power_on_delay_ms)
+ msleep(conf->post_power_on_delay_ms);
+
+ return 0;
+}
+
+static int spi_hid_acpi_assert_reset(struct spihid_ops *ops)
+{
+ return 0;
+}
+
+static int spi_hid_acpi_deassert_reset(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+
+ return device_reset(&conf->adev->dev);
+}
+
+static void spi_hid_acpi_sleep_minimal_reset_delay(struct spihid_ops *ops)
+{
+ struct spi_hid_acpi_config *conf = container_of(ops,
+ struct spi_hid_acpi_config,
+ ops);
+ usleep_range(1000 * conf->minimal_reset_delay_ms,
+ 1000 * (conf->minimal_reset_delay_ms + 1));
+}
+
+static int spi_hid_acpi_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct acpi_device *adev;
+ struct spi_hid_acpi_config *config;
+ int error;
+
+ adev = ACPI_COMPANION(dev);
+ if (!adev) {
+ dev_err(dev, "Error could not get ACPI device.");
+ return -ENODEV;
+ }
+
+ config = devm_kzalloc(dev, sizeof(struct spi_hid_acpi_config),
+ GFP_KERNEL);
+ if (!config)
+ return -ENOMEM;
+
+ if (acpi_device_power_manageable(adev)) {
+ config->ops.power_up = spi_hid_acpi_power_up;
+ config->ops.power_down = spi_hid_acpi_power_down;
+ } else {
+ config->ops.power_up = spi_hid_acpi_power_none;
+ config->ops.power_down = spi_hid_acpi_power_none;
+ }
+ config->ops.assert_reset = spi_hid_acpi_assert_reset;
+ config->ops.deassert_reset = spi_hid_acpi_deassert_reset;
+ config->ops.sleep_minimal_reset_delay =
+ spi_hid_acpi_sleep_minimal_reset_delay;
+
+ error = spi_hid_acpi_populate_config(config, adev);
+ if (error) {
+ dev_err(dev, "%s: unable to populate config data.", __func__);
+ return error;
+ }
+ return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
+}
+
+static const struct acpi_device_id spi_hid_acpi_match[] = {
+ { "ACPI0C51", 0 },
+ { "PNP0C51", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, spi_hid_acpi_match);
+
+static struct spi_driver spi_hid_acpi_driver = {
+ .driver = {
+ .name = "spi_hid_acpi",
+ .owner = THIS_MODULE,
+ .acpi_match_table = ACPI_PTR(spi_hid_acpi_match),
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .dev_groups = spi_hid_groups,
+ },
+ .probe = spi_hid_acpi_probe,
+ .remove = spi_hid_core_remove,
+};
+
+module_spi_driver(spi_hid_acpi_driver);
+
+MODULE_DESCRIPTION("HID over SPI ACPI transport driver");
+MODULE_AUTHOR("Angela Czubak <aczubak@google.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index e3273846267e..02beb209a92d 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -43,6 +43,9 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include "spi-hid.h"
+#include "spi-hid-core.h"
+
/* Protocol constants */
#define SPI_HID_READ_APPROVAL_CONSTANT 0xff
#define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
@@ -105,30 +108,6 @@ struct spi_hid_output_report {
u8 *content;
};
-/* struct spi_hid_conf - Conf provided to the core */
-struct spi_hid_conf {
- u32 input_report_header_address;
- u32 input_report_body_address;
- u32 output_report_address;
- u8 read_opcode;
- u8 write_opcode;
-};
-
-/**
- * struct spihid_ops - Ops provided to the core
- * @power_up: do sequencing to power up the device
- * @power_down: do sequencing to power down the device
- * @assert_reset: do sequencing to assert the reset line
- * @deassert_reset: do sequencing to deassert the reset line
- */
-struct spihid_ops {
- int (*power_up)(struct spihid_ops *ops);
- int (*power_down)(struct spihid_ops *ops);
- int (*assert_reset)(struct spihid_ops *ops);
- int (*deassert_reset)(struct spihid_ops *ops);
- void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
-};
-
static struct hid_ll_driver spi_hid_ll_driver;
static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
new file mode 100644
index 000000000000..1fdd45262647
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Microsoft Corporation
+ * Copyright (c) 2026 Google LLC
+ */
+
+#ifndef SPI_HID_H
+#define SPI_HID_H
+
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+
+/* struct spi_hid_conf - Conf provided to the core */
+struct spi_hid_conf {
+ u32 input_report_header_address;
+ u32 input_report_body_address;
+ u32 output_report_address;
+ u8 read_opcode;
+ u8 write_opcode;
+};
+
+/**
+ * struct spihid_ops - Ops provided to the core
+ * @power_up: do sequencing to power up the device
+ * @power_down: do sequencing to power down the device
+ * @assert_reset: do sequencing to assert the reset line
+ * @deassert_reset: do sequencing to deassert the reset line
+ */
+struct spihid_ops {
+ int (*power_up)(struct spihid_ops *ops);
+ int (*power_down)(struct spihid_ops *ops);
+ int (*assert_reset)(struct spihid_ops *ops);
+ int (*deassert_reset)(struct spihid_ops *ops);
+ void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
+};
+
+int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
+ struct spi_hid_conf *conf);
+
+void spi_hid_core_remove(struct spi_device *spi);
+
+extern const struct attribute_group *spi_hid_groups[];
+
+#endif /* SPI_HID_H */
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 07/12] HID: spi_hid: add ACPI support for SPI over HID
2026-03-03 6:12 ` [PATCH 07/12] HID: spi_hid: add ACPI support for SPI over HID Jingyuan Liang
@ 2026-03-11 5:27 ` Dmitry Torokhov
2026-03-13 1:24 ` Jingyuan Liang
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Torokhov @ 2026-03-11 5:27 UTC (permalink / raw)
To: Jingyuan Liang
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, linux-doc,
linux-kernel, linux-spi, linux-trace-kernel, devicetree, hbarnor,
Angela Czubak
On Tue, Mar 03, 2026 at 06:12:59AM +0000, Jingyuan Liang wrote:
> From: Angela Czubak <acz@semihalf.com>
>
> Detect SPI HID devices described in ACPI.
>
> Signed-off-by: Angela Czubak <acz@semihalf.com>
> Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> ---
> drivers/hid/spi-hid/Kconfig | 15 +++
> drivers/hid/spi-hid/Makefile | 1 +
> drivers/hid/spi-hid/spi-hid-acpi.c | 253 +++++++++++++++++++++++++++++++++++++
> drivers/hid/spi-hid/spi-hid-core.c | 27 +---
> drivers/hid/spi-hid/spi-hid.h | 44 +++++++
> 5 files changed, 316 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
> index 836fdefe8345..114b1e00da39 100644
> --- a/drivers/hid/spi-hid/Kconfig
> +++ b/drivers/hid/spi-hid/Kconfig
> @@ -10,6 +10,21 @@ menuconfig SPI_HID
>
> if SPI_HID
>
> +config SPI_HID_ACPI
> + tristate "HID over SPI transport layer ACPI driver"
> + depends on ACPI
> + select SPI_HID_CORE
> + help
> + Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
> + other HID based devices which are connected to your computer via SPI.
> + This driver supports ACPI-based systems.
> +
> + If unsure, say N.
> +
> + This support is also available as a module. If so, the module
> + will be called spi-hid-acpi. It will also build/depend on the
> + module spi-hid.
> +
> config SPI_HID_CORE
> tristate
> endif
> diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
> index 92e24cddbfc2..753c7b7a7844 100644
> --- a/drivers/hid/spi-hid/Makefile
> +++ b/drivers/hid/spi-hid/Makefile
> @@ -7,3 +7,4 @@
>
> obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
> spi-hid-objs = spi-hid-core.o
> +obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
> diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
> new file mode 100644
> index 000000000000..612e74fe72f9
> --- /dev/null
> +++ b/drivers/hid/spi-hid/spi-hid-acpi.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * HID over SPI protocol, ACPI related code
> + *
> + * Copyright (c) 2021 Microsoft Corporation
> + * Copyright (c) 2026 Google LLC
> + *
> + * This code was forked out of the HID over SPI core code, which is partially
> + * based on "HID over I2C protocol implementation:
> + *
> + * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
> + * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
> + * Copyright (c) 2012 Red Hat, Inc
> + *
> + * which in turn is partially based on "USB HID support for Linux":
> + *
> + * Copyright (c) 1999 Andreas Gal
> + * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
> + * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
> + * Copyright (c) 2007-2008 Oliver Neukum
> + * Copyright (c) 2006-2010 Jiri Kosina
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/reset.h>
> +#include <linux/uuid.h>
> +
> +#include "spi-hid.h"
> +
> +/* Config structure is filled with data from ACPI */
> +struct spi_hid_acpi_config {
> + struct spihid_ops ops;
> +
> + struct spi_hid_conf property_conf;
> + u32 post_power_on_delay_ms;
> + u32 minimal_reset_delay_ms;
> + struct acpi_device *adev;
> +};
> +
> +/* HID SPI Device: 6e2ac436-0fcf41af-a265-b32a220dcfab */
> +static guid_t spi_hid_guid =
> + GUID_INIT(0x6E2AC436, 0x0FCF, 0x41AF,
> + 0xA2, 0x65, 0xB3, 0x2A, 0x22, 0x0D, 0xCF, 0xAB);
> +
> +static int spi_hid_acpi_populate_config(struct spi_hid_acpi_config *conf,
> + struct acpi_device *adev)
> +{
> + acpi_handle handle = acpi_device_handle(adev);
> + union acpi_object *obj;
> +
> + conf->adev = adev;
> +
> + /* Revision 3 for HID over SPI V1, see specification. */
> + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 1, NULL,
> + ACPI_TYPE_INTEGER);
> + if (!obj) {
> + acpi_handle_err(handle,
> + "Error _DSM call to get HID input report header address failed");
> + return -ENODEV;
> + }
> + conf->property_conf.input_report_header_address = obj->integer.value;
> + ACPI_FREE(obj);
> +
> + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 2, NULL,
> + ACPI_TYPE_INTEGER);
> + if (!obj) {
> + acpi_handle_err(handle,
> + "Error _DSM call to get HID input report body address failed");
> + return -ENODEV;
> + }
> + conf->property_conf.input_report_body_address = obj->integer.value;
> + ACPI_FREE(obj);
> +
> + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 3, NULL,
> + ACPI_TYPE_INTEGER);
> + if (!obj) {
> + acpi_handle_err(handle,
> + "Error _DSM call to get HID output report header address failed");
> + return -ENODEV;
> + }
> + conf->property_conf.output_report_address = obj->integer.value;
> + ACPI_FREE(obj);
> +
> + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 4, NULL,
> + ACPI_TYPE_BUFFER);
> + if (!obj) {
> + acpi_handle_err(handle,
> + "Error _DSM call to get HID read opcode failed");
> + return -ENODEV;
> + }
> + if (obj->buffer.length == 1) {
> + conf->property_conf.read_opcode = obj->buffer.pointer[0];
> + } else {
> + acpi_handle_err(handle,
> + "Error _DSM call to get HID read opcode, too long buffer");
> + ACPI_FREE(obj);
> + return -ENODEV;
> + }
> + ACPI_FREE(obj);
> +
> + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 5, NULL,
> + ACPI_TYPE_BUFFER);
> + if (!obj) {
> + acpi_handle_err(handle,
> + "Error _DSM call to get HID write opcode failed");
> + return -ENODEV;
> + }
> + if (obj->buffer.length == 1) {
> + conf->property_conf.write_opcode = obj->buffer.pointer[0];
> + } else {
> + acpi_handle_err(handle,
> + "Error _DSM call to get HID write opcode, too long buffer");
> + ACPI_FREE(obj);
> + return -ENODEV;
> + }
> + ACPI_FREE(obj);
> +
> + /* Value not provided in ACPI,*/
> + conf->post_power_on_delay_ms = 5;
> + conf->minimal_reset_delay_ms = 150;
> +
> + if (!acpi_has_method(handle, "_RST")) {
> + acpi_handle_err(handle, "No reset method for acpi handle");
> + return -ENODEV;
I would return -EINVAL as we have the device with right _DSM but without
mandated by the spec _RST.
> + }
> +
> + /* FIXME: not reading hid-over-spi-flags, multi-SPI not supported */
> +
> + return 0;
> +}
> +
> +static int spi_hid_acpi_power_none(struct spihid_ops *ops)
> +{
> + return 0;
> +}
> +
> +static int spi_hid_acpi_power_down(struct spihid_ops *ops)
> +{
> + struct spi_hid_acpi_config *conf = container_of(ops,
> + struct spi_hid_acpi_config,
> + ops);
> +
> + return acpi_device_set_power(conf->adev, ACPI_STATE_D3);
> +}
> +
> +static int spi_hid_acpi_power_up(struct spihid_ops *ops)
> +{
> + struct spi_hid_acpi_config *conf = container_of(ops,
> + struct spi_hid_acpi_config,
> + ops);
> + int error;
> +
> + error = acpi_device_set_power(conf->adev, ACPI_STATE_D0);
> + if (error) {
> + dev_err(&conf->adev->dev, "Error could not power up ACPI device: %d.", error);
> + return error;
> + }
> +
> + if (conf->post_power_on_delay_ms)
> + msleep(conf->post_power_on_delay_ms);
> +
> + return 0;
> +}
> +
> +static int spi_hid_acpi_assert_reset(struct spihid_ops *ops)
> +{
> + return 0;
> +}
> +
> +static int spi_hid_acpi_deassert_reset(struct spihid_ops *ops)
> +{
> + struct spi_hid_acpi_config *conf = container_of(ops,
> + struct spi_hid_acpi_config,
> + ops);
> +
> + return device_reset(&conf->adev->dev);
> +}
> +
> +static void spi_hid_acpi_sleep_minimal_reset_delay(struct spihid_ops *ops)
> +{
> + struct spi_hid_acpi_config *conf = container_of(ops,
> + struct spi_hid_acpi_config,
> + ops);
> + usleep_range(1000 * conf->minimal_reset_delay_ms,
> + 1000 * (conf->minimal_reset_delay_ms + 1));
I'd probably use "fsleep(conf->minimal_reset_delay_ms * 1000)".
> +}
> +
> +static int spi_hid_acpi_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct acpi_device *adev;
> + struct spi_hid_acpi_config *config;
> + int error;
> +
> + adev = ACPI_COMPANION(dev);
> + if (!adev) {
> + dev_err(dev, "Error could not get ACPI device.");
> + return -ENODEV;
> + }
> +
> + config = devm_kzalloc(dev, sizeof(struct spi_hid_acpi_config),
> + GFP_KERNEL);
> + if (!config)
> + return -ENOMEM;
> +
> + if (acpi_device_power_manageable(adev)) {
> + config->ops.power_up = spi_hid_acpi_power_up;
> + config->ops.power_down = spi_hid_acpi_power_down;
> + } else {
> + config->ops.power_up = spi_hid_acpi_power_none;
> + config->ops.power_down = spi_hid_acpi_power_none;
> + }
> + config->ops.assert_reset = spi_hid_acpi_assert_reset;
> + config->ops.deassert_reset = spi_hid_acpi_deassert_reset;
> + config->ops.sleep_minimal_reset_delay =
> + spi_hid_acpi_sleep_minimal_reset_delay;
> +
> + error = spi_hid_acpi_populate_config(config, adev);
> + if (error) {
> + dev_err(dev, "%s: unable to populate config data.", __func__);
> + return error;
> + }
I would add a blank line.
> + return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
> +}
> +
> +static const struct acpi_device_id spi_hid_acpi_match[] = {
> + { "ACPI0C51", 0 },
> + { "PNP0C51", 0 },
> + { },
No comma on sentinels.
> +};
> +MODULE_DEVICE_TABLE(acpi, spi_hid_acpi_match);
> +
> +static struct spi_driver spi_hid_acpi_driver = {
> + .driver = {
> + .name = "spi_hid_acpi",
> + .owner = THIS_MODULE,
> + .acpi_match_table = ACPI_PTR(spi_hid_acpi_match),
This is dependent on ACPI, so no need to sue ACPI_PTR().
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + .dev_groups = spi_hid_groups,
> + },
> + .probe = spi_hid_acpi_probe,
> + .remove = spi_hid_core_remove,
> +};
> +
> +module_spi_driver(spi_hid_acpi_driver);
> +
> +MODULE_DESCRIPTION("HID over SPI ACPI transport driver");
> +MODULE_AUTHOR("Angela Czubak <aczubak@google.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
> index e3273846267e..02beb209a92d 100644
> --- a/drivers/hid/spi-hid/spi-hid-core.c
> +++ b/drivers/hid/spi-hid/spi-hid-core.c
> @@ -43,6 +43,9 @@
> #include <linux/wait.h>
> #include <linux/workqueue.h>
>
> +#include "spi-hid.h"
> +#include "spi-hid-core.h"
> +
> /* Protocol constants */
> #define SPI_HID_READ_APPROVAL_CONSTANT 0xff
> #define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
> @@ -105,30 +108,6 @@ struct spi_hid_output_report {
> u8 *content;
> };
>
> -/* struct spi_hid_conf - Conf provided to the core */
> -struct spi_hid_conf {
> - u32 input_report_header_address;
> - u32 input_report_body_address;
> - u32 output_report_address;
> - u8 read_opcode;
> - u8 write_opcode;
> -};
> -
> -/**
> - * struct spihid_ops - Ops provided to the core
> - * @power_up: do sequencing to power up the device
> - * @power_down: do sequencing to power down the device
> - * @assert_reset: do sequencing to assert the reset line
> - * @deassert_reset: do sequencing to deassert the reset line
> - */
> -struct spihid_ops {
> - int (*power_up)(struct spihid_ops *ops);
> - int (*power_down)(struct spihid_ops *ops);
> - int (*assert_reset)(struct spihid_ops *ops);
> - int (*deassert_reset)(struct spihid_ops *ops);
> - void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
> -};
> -
> static struct hid_ll_driver spi_hid_ll_driver;
>
> static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
> diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
> new file mode 100644
> index 000000000000..1fdd45262647
> --- /dev/null
> +++ b/drivers/hid/spi-hid/spi-hid.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2021 Microsoft Corporation
> + * Copyright (c) 2026 Google LLC
> + */
> +
> +#ifndef SPI_HID_H
> +#define SPI_HID_H
> +
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +
> +/* struct spi_hid_conf - Conf provided to the core */
> +struct spi_hid_conf {
> + u32 input_report_header_address;
> + u32 input_report_body_address;
> + u32 output_report_address;
> + u8 read_opcode;
> + u8 write_opcode;
> +};
> +
> +/**
> + * struct spihid_ops - Ops provided to the core
> + * @power_up: do sequencing to power up the device
> + * @power_down: do sequencing to power down the device
> + * @assert_reset: do sequencing to assert the reset line
> + * @deassert_reset: do sequencing to deassert the reset line
> + */
> +struct spihid_ops {
> + int (*power_up)(struct spihid_ops *ops);
> + int (*power_down)(struct spihid_ops *ops);
> + int (*assert_reset)(struct spihid_ops *ops);
> + int (*deassert_reset)(struct spihid_ops *ops);
> + void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
> +};
> +
> +int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
> + struct spi_hid_conf *conf);
> +
> +void spi_hid_core_remove(struct spi_device *spi);
> +
> +extern const struct attribute_group *spi_hid_groups[];
> +
> +#endif /* SPI_HID_H */
I am not sure if this belongs to this patch or if it should be better in
the patch introducing the main driver from the beginning.
For the ACPI part:
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 07/12] HID: spi_hid: add ACPI support for SPI over HID
2026-03-11 5:27 ` Dmitry Torokhov
@ 2026-03-13 1:24 ` Jingyuan Liang
0 siblings, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-13 1:24 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, linux-doc,
linux-kernel, linux-spi, linux-trace-kernel, devicetree, hbarnor,
Angela Czubak
On Tue, Mar 10, 2026 at 10:27 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Tue, Mar 03, 2026 at 06:12:59AM +0000, Jingyuan Liang wrote:
> > From: Angela Czubak <acz@semihalf.com>
> >
> > Detect SPI HID devices described in ACPI.
> >
> > Signed-off-by: Angela Czubak <acz@semihalf.com>
> > Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> > ---
> > drivers/hid/spi-hid/Kconfig | 15 +++
> > drivers/hid/spi-hid/Makefile | 1 +
> > drivers/hid/spi-hid/spi-hid-acpi.c | 253 +++++++++++++++++++++++++++++++++++++
> > drivers/hid/spi-hid/spi-hid-core.c | 27 +---
> > drivers/hid/spi-hid/spi-hid.h | 44 +++++++
> > 5 files changed, 316 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
> > index 836fdefe8345..114b1e00da39 100644
> > --- a/drivers/hid/spi-hid/Kconfig
> > +++ b/drivers/hid/spi-hid/Kconfig
> > @@ -10,6 +10,21 @@ menuconfig SPI_HID
> >
> > if SPI_HID
> >
> > +config SPI_HID_ACPI
> > + tristate "HID over SPI transport layer ACPI driver"
> > + depends on ACPI
> > + select SPI_HID_CORE
> > + help
> > + Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
> > + other HID based devices which are connected to your computer via SPI.
> > + This driver supports ACPI-based systems.
> > +
> > + If unsure, say N.
> > +
> > + This support is also available as a module. If so, the module
> > + will be called spi-hid-acpi. It will also build/depend on the
> > + module spi-hid.
> > +
> > config SPI_HID_CORE
> > tristate
> > endif
> > diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
> > index 92e24cddbfc2..753c7b7a7844 100644
> > --- a/drivers/hid/spi-hid/Makefile
> > +++ b/drivers/hid/spi-hid/Makefile
> > @@ -7,3 +7,4 @@
> >
> > obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
> > spi-hid-objs = spi-hid-core.o
> > +obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
> > diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
> > new file mode 100644
> > index 000000000000..612e74fe72f9
> > --- /dev/null
> > +++ b/drivers/hid/spi-hid/spi-hid-acpi.c
> > @@ -0,0 +1,253 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * HID over SPI protocol, ACPI related code
> > + *
> > + * Copyright (c) 2021 Microsoft Corporation
> > + * Copyright (c) 2026 Google LLC
> > + *
> > + * This code was forked out of the HID over SPI core code, which is partially
> > + * based on "HID over I2C protocol implementation:
> > + *
> > + * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
> > + * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
> > + * Copyright (c) 2012 Red Hat, Inc
> > + *
> > + * which in turn is partially based on "USB HID support for Linux":
> > + *
> > + * Copyright (c) 1999 Andreas Gal
> > + * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
> > + * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
> > + * Copyright (c) 2007-2008 Oliver Neukum
> > + * Copyright (c) 2006-2010 Jiri Kosina
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/reset.h>
> > +#include <linux/uuid.h>
> > +
> > +#include "spi-hid.h"
> > +
> > +/* Config structure is filled with data from ACPI */
> > +struct spi_hid_acpi_config {
> > + struct spihid_ops ops;
> > +
> > + struct spi_hid_conf property_conf;
> > + u32 post_power_on_delay_ms;
> > + u32 minimal_reset_delay_ms;
> > + struct acpi_device *adev;
> > +};
> > +
> > +/* HID SPI Device: 6e2ac436-0fcf41af-a265-b32a220dcfab */
> > +static guid_t spi_hid_guid =
> > + GUID_INIT(0x6E2AC436, 0x0FCF, 0x41AF,
> > + 0xA2, 0x65, 0xB3, 0x2A, 0x22, 0x0D, 0xCF, 0xAB);
> > +
> > +static int spi_hid_acpi_populate_config(struct spi_hid_acpi_config *conf,
> > + struct acpi_device *adev)
> > +{
> > + acpi_handle handle = acpi_device_handle(adev);
> > + union acpi_object *obj;
> > +
> > + conf->adev = adev;
> > +
> > + /* Revision 3 for HID over SPI V1, see specification. */
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 1, NULL,
> > + ACPI_TYPE_INTEGER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID input report header address failed");
> > + return -ENODEV;
> > + }
> > + conf->property_conf.input_report_header_address = obj->integer.value;
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 2, NULL,
> > + ACPI_TYPE_INTEGER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID input report body address failed");
> > + return -ENODEV;
> > + }
> > + conf->property_conf.input_report_body_address = obj->integer.value;
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 3, NULL,
> > + ACPI_TYPE_INTEGER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID output report header address failed");
> > + return -ENODEV;
> > + }
> > + conf->property_conf.output_report_address = obj->integer.value;
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 4, NULL,
> > + ACPI_TYPE_BUFFER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID read opcode failed");
> > + return -ENODEV;
> > + }
> > + if (obj->buffer.length == 1) {
> > + conf->property_conf.read_opcode = obj->buffer.pointer[0];
> > + } else {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID read opcode, too long buffer");
> > + ACPI_FREE(obj);
> > + return -ENODEV;
> > + }
> > + ACPI_FREE(obj);
> > +
> > + obj = acpi_evaluate_dsm_typed(handle, &spi_hid_guid, 3, 5, NULL,
> > + ACPI_TYPE_BUFFER);
> > + if (!obj) {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID write opcode failed");
> > + return -ENODEV;
> > + }
> > + if (obj->buffer.length == 1) {
> > + conf->property_conf.write_opcode = obj->buffer.pointer[0];
> > + } else {
> > + acpi_handle_err(handle,
> > + "Error _DSM call to get HID write opcode, too long buffer");
> > + ACPI_FREE(obj);
> > + return -ENODEV;
> > + }
> > + ACPI_FREE(obj);
> > +
> > + /* Value not provided in ACPI,*/
> > + conf->post_power_on_delay_ms = 5;
> > + conf->minimal_reset_delay_ms = 150;
> > +
> > + if (!acpi_has_method(handle, "_RST")) {
> > + acpi_handle_err(handle, "No reset method for acpi handle");
> > + return -ENODEV;
>
> I would return -EINVAL as we have the device with right _DSM but without
> mandated by the spec _RST.
Thanks! Will fix this in v2.
>
> > + }
> > +
> > + /* FIXME: not reading hid-over-spi-flags, multi-SPI not supported */
> > +
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_power_none(struct spihid_ops *ops)
> > +{
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_power_down(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > +
> > + return acpi_device_set_power(conf->adev, ACPI_STATE_D3);
> > +}
> > +
> > +static int spi_hid_acpi_power_up(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > + int error;
> > +
> > + error = acpi_device_set_power(conf->adev, ACPI_STATE_D0);
> > + if (error) {
> > + dev_err(&conf->adev->dev, "Error could not power up ACPI device: %d.", error);
> > + return error;
> > + }
> > +
> > + if (conf->post_power_on_delay_ms)
> > + msleep(conf->post_power_on_delay_ms);
> > +
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_assert_reset(struct spihid_ops *ops)
> > +{
> > + return 0;
> > +}
> > +
> > +static int spi_hid_acpi_deassert_reset(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > +
> > + return device_reset(&conf->adev->dev);
> > +}
> > +
> > +static void spi_hid_acpi_sleep_minimal_reset_delay(struct spihid_ops *ops)
> > +{
> > + struct spi_hid_acpi_config *conf = container_of(ops,
> > + struct spi_hid_acpi_config,
> > + ops);
> > + usleep_range(1000 * conf->minimal_reset_delay_ms,
> > + 1000 * (conf->minimal_reset_delay_ms + 1));
>
> I'd probably use "fsleep(conf->minimal_reset_delay_ms * 1000)".
I will fix this in v2. And do the same for the of driver.
>
> > +}
> > +
> > +static int spi_hid_acpi_probe(struct spi_device *spi)
> > +{
> > + struct device *dev = &spi->dev;
> > + struct acpi_device *adev;
> > + struct spi_hid_acpi_config *config;
> > + int error;
> > +
> > + adev = ACPI_COMPANION(dev);
> > + if (!adev) {
> > + dev_err(dev, "Error could not get ACPI device.");
> > + return -ENODEV;
> > + }
> > +
> > + config = devm_kzalloc(dev, sizeof(struct spi_hid_acpi_config),
> > + GFP_KERNEL);
> > + if (!config)
> > + return -ENOMEM;
> > +
> > + if (acpi_device_power_manageable(adev)) {
> > + config->ops.power_up = spi_hid_acpi_power_up;
> > + config->ops.power_down = spi_hid_acpi_power_down;
> > + } else {
> > + config->ops.power_up = spi_hid_acpi_power_none;
> > + config->ops.power_down = spi_hid_acpi_power_none;
> > + }
> > + config->ops.assert_reset = spi_hid_acpi_assert_reset;
> > + config->ops.deassert_reset = spi_hid_acpi_deassert_reset;
> > + config->ops.sleep_minimal_reset_delay =
> > + spi_hid_acpi_sleep_minimal_reset_delay;
> > +
> > + error = spi_hid_acpi_populate_config(config, adev);
> > + if (error) {
> > + dev_err(dev, "%s: unable to populate config data.", __func__);
> > + return error;
> > + }
>
> I would add a blank line.
Sure! Will fix this in v2.
>
> > + return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
> > +}
> > +
> > +static const struct acpi_device_id spi_hid_acpi_match[] = {
> > + { "ACPI0C51", 0 },
> > + { "PNP0C51", 0 },
> > + { },
>
> No comma on sentinels.
Will fix this in v2.
>
> > +};
> > +MODULE_DEVICE_TABLE(acpi, spi_hid_acpi_match);
> > +
> > +static struct spi_driver spi_hid_acpi_driver = {
> > + .driver = {
> > + .name = "spi_hid_acpi",
> > + .owner = THIS_MODULE,
> > + .acpi_match_table = ACPI_PTR(spi_hid_acpi_match),
>
> This is dependent on ACPI, so no need to sue ACPI_PTR().
Will fix this in v2 and remove of_match_ptr in the of driver as well.
>
> > + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> > + .dev_groups = spi_hid_groups,
> > + },
> > + .probe = spi_hid_acpi_probe,
> > + .remove = spi_hid_core_remove,
> > +};
> > +
> > +module_spi_driver(spi_hid_acpi_driver);
> > +
> > +MODULE_DESCRIPTION("HID over SPI ACPI transport driver");
> > +MODULE_AUTHOR("Angela Czubak <aczubak@google.com>");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
> > index e3273846267e..02beb209a92d 100644
> > --- a/drivers/hid/spi-hid/spi-hid-core.c
> > +++ b/drivers/hid/spi-hid/spi-hid-core.c
> > @@ -43,6 +43,9 @@
> > #include <linux/wait.h>
> > #include <linux/workqueue.h>
> >
> > +#include "spi-hid.h"
> > +#include "spi-hid-core.h"
> > +
> > /* Protocol constants */
> > #define SPI_HID_READ_APPROVAL_CONSTANT 0xff
> > #define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
> > @@ -105,30 +108,6 @@ struct spi_hid_output_report {
> > u8 *content;
> > };
> >
> > -/* struct spi_hid_conf - Conf provided to the core */
> > -struct spi_hid_conf {
> > - u32 input_report_header_address;
> > - u32 input_report_body_address;
> > - u32 output_report_address;
> > - u8 read_opcode;
> > - u8 write_opcode;
> > -};
> > -
> > -/**
> > - * struct spihid_ops - Ops provided to the core
> > - * @power_up: do sequencing to power up the device
> > - * @power_down: do sequencing to power down the device
> > - * @assert_reset: do sequencing to assert the reset line
> > - * @deassert_reset: do sequencing to deassert the reset line
> > - */
> > -struct spihid_ops {
> > - int (*power_up)(struct spihid_ops *ops);
> > - int (*power_down)(struct spihid_ops *ops);
> > - int (*assert_reset)(struct spihid_ops *ops);
> > - int (*deassert_reset)(struct spihid_ops *ops);
> > - void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
> > -};
> > -
> > static struct hid_ll_driver spi_hid_ll_driver;
> >
> > static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
> > diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
> > new file mode 100644
> > index 000000000000..1fdd45262647
> > --- /dev/null
> > +++ b/drivers/hid/spi-hid/spi-hid.h
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2021 Microsoft Corporation
> > + * Copyright (c) 2026 Google LLC
> > + */
> > +
> > +#ifndef SPI_HID_H
> > +#define SPI_HID_H
> > +
> > +#include <linux/spi/spi.h>
> > +#include <linux/sysfs.h>
> > +
> > +/* struct spi_hid_conf - Conf provided to the core */
> > +struct spi_hid_conf {
> > + u32 input_report_header_address;
> > + u32 input_report_body_address;
> > + u32 output_report_address;
> > + u8 read_opcode;
> > + u8 write_opcode;
> > +};
> > +
> > +/**
> > + * struct spihid_ops - Ops provided to the core
> > + * @power_up: do sequencing to power up the device
> > + * @power_down: do sequencing to power down the device
> > + * @assert_reset: do sequencing to assert the reset line
> > + * @deassert_reset: do sequencing to deassert the reset line
> > + */
> > +struct spihid_ops {
> > + int (*power_up)(struct spihid_ops *ops);
> > + int (*power_down)(struct spihid_ops *ops);
> > + int (*assert_reset)(struct spihid_ops *ops);
> > + int (*deassert_reset)(struct spihid_ops *ops);
> > + void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
> > +};
> > +
> > +int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
> > + struct spi_hid_conf *conf);
> > +
> > +void spi_hid_core_remove(struct spi_device *spi);
> > +
> > +extern const struct attribute_group *spi_hid_groups[];
> > +
> > +#endif /* SPI_HID_H */
>
> I am not sure if this belongs to this patch or if it should be better in
> the patch introducing the main driver from the beginning.
These definitions were in spi-hid-core.c in the previous patch introducing
the main driver because it was only used in one .c file. This patch introduces
spi-hid-acpi.c and now two .c files need it so I created a separate .h
file here.
>
> For the ACPI part:
>
> Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 08/12] HID: spi_hid: add device tree support for SPI over HID
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
` (6 preceding siblings ...)
2026-03-03 6:12 ` [PATCH 07/12] HID: spi_hid: add ACPI support for SPI over HID Jingyuan Liang
@ 2026-03-03 6:13 ` Jingyuan Liang
2026-03-03 6:13 ` [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema Jingyuan Liang
` (3 subsequent siblings)
11 siblings, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Jarrett Schultz, Dmitry Antipov
From: Jarrett Schultz <jaschultz@microsoft.com>
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/Kconfig | 15 +++
drivers/hid/spi-hid/Makefile | 1 +
drivers/hid/spi-hid/spi-hid-of.c | 243 +++++++++++++++++++++++++++++++++++++++
3 files changed, 259 insertions(+)
diff --git a/drivers/hid/spi-hid/Kconfig b/drivers/hid/spi-hid/Kconfig
index 114b1e00da39..76a2cd587a3e 100644
--- a/drivers/hid/spi-hid/Kconfig
+++ b/drivers/hid/spi-hid/Kconfig
@@ -25,6 +25,21 @@ config SPI_HID_ACPI
will be called spi-hid-acpi. It will also build/depend on the
module spi-hid.
+config SPI_HID_OF
+ tristate "HID over SPI transport layer Open Firmware driver"
+ depends on OF
+ select SPI_HID_CORE
+ help
+ Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
+ other HID based devices which are connected to your computer via SPI.
+ This driver supports Open Firmware (Device Tree)-based systems.
+
+ If unsure, say N.
+
+ This support is also available as a module. If so, the module
+ will be called spi-hid-of. It will also build/depend on the
+ module spi-hid.
+
config SPI_HID_CORE
tristate
endif
diff --git a/drivers/hid/spi-hid/Makefile b/drivers/hid/spi-hid/Makefile
index 753c7b7a7844..fe627fd378e3 100644
--- a/drivers/hid/spi-hid/Makefile
+++ b/drivers/hid/spi-hid/Makefile
@@ -8,3 +8,4 @@
obj-$(CONFIG_SPI_HID_CORE) += spi-hid.o
spi-hid-objs = spi-hid-core.o
obj-$(CONFIG_SPI_HID_ACPI) += spi-hid-acpi.o
+obj-$(CONFIG_SPI_HID_OF) += spi-hid-of.o
diff --git a/drivers/hid/spi-hid/spi-hid-of.c b/drivers/hid/spi-hid/spi-hid-of.c
new file mode 100644
index 000000000000..a20c8146230b
--- /dev/null
+++ b/drivers/hid/spi-hid/spi-hid-of.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID over SPI protocol, Open Firmware related code
+ *
+ * Copyright (c) 2021 Microsoft Corporation
+ *
+ * This code was forked out of the HID over SPI core code, which is partially
+ * based on "HID over I2C protocol implementation:
+ *
+ * Copyright (c) 2012 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * Copyright (c) 2012 Ecole Nationale de l'Aviation Civile, France
+ * Copyright (c) 2012 Red Hat, Inc
+ *
+ * which in turn is partially based on "USB HID support for Linux":
+ *
+ * Copyright (c) 1999 Andreas Gal
+ * Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
+ * Copyright (c) 2007-2008 Oliver Neukum
+ * Copyright (c) 2006-2010 Jiri Kosina
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+
+#include "spi-hid.h"
+
+/* Config structure is filled with data from Device Tree */
+struct spi_hid_of_config {
+ struct spihid_ops ops;
+
+ struct spi_hid_conf property_conf;
+ u32 post_power_on_delay_ms;
+ u32 minimal_reset_delay_ms;
+ struct gpio_desc *reset_gpio;
+ struct regulator *supply;
+ bool supply_enabled;
+};
+
+static int spi_hid_of_populate_config(struct spi_hid_of_config *conf,
+ struct device *dev)
+{
+ int error;
+ u32 val;
+
+ error = device_property_read_u32(dev, "input-report-header-address",
+ &val);
+ if (error) {
+ dev_err(dev, "Input report header address not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_header_address = val;
+
+ error = device_property_read_u32(dev, "input-report-body-address", &val);
+ if (error) {
+ dev_err(dev, "Input report body address not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.input_report_body_address = val;
+
+ error = device_property_read_u32(dev, "output-report-address", &val);
+ if (error) {
+ dev_err(dev, "Output report address not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.output_report_address = val;
+
+ error = device_property_read_u32(dev, "read-opcode", &val);
+ if (error) {
+ dev_err(dev, "Read opcode not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.read_opcode = val;
+
+ error = device_property_read_u32(dev, "write-opcode", &val);
+ if (error) {
+ dev_err(dev, "Write opcode not provided.");
+ return -ENODEV;
+ }
+ conf->property_conf.write_opcode = val;
+
+ error = device_property_read_u32(dev, "post-power-on-delay-ms", &val);
+ if (error) {
+ dev_err(dev, "post-power-on-delay-ms not provided, using 10.");
+ val = 10;
+ }
+ conf->post_power_on_delay_ms = val;
+
+ error = device_property_read_u32(dev, "minimal-reset-delay-ms", &val);
+ if (error) {
+ dev_err(dev, "minimal-reset-delay-ms not provided, using 100.");
+ val = 100;
+ }
+ conf->minimal_reset_delay_ms = val;
+
+ /* FIXME: not reading hid-over-spi-flags, multi-fragment not supported */
+
+ conf->supply = devm_regulator_get(dev, "vdd");
+ if (IS_ERR(conf->supply)) {
+ if (PTR_ERR(conf->supply) != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get regulator: %ld.",
+ PTR_ERR(conf->supply));
+ return PTR_ERR(conf->supply);
+ }
+ conf->supply_enabled = false;
+
+ conf->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(conf->reset_gpio)) {
+ dev_err(dev, "%s: error getting reset GPIO.", __func__);
+ return PTR_ERR(conf->reset_gpio);
+ }
+
+ return 0;
+}
+
+static int spi_hid_of_power_down(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+ int error;
+
+ if (!conf->supply_enabled)
+ return 0;
+
+ error = regulator_disable(conf->supply);
+ if (error == 0)
+ conf->supply_enabled = false;
+
+ return error;
+}
+
+static int spi_hid_of_power_up(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+ int error;
+
+ if (conf->supply_enabled)
+ return 0;
+
+ error = regulator_enable(conf->supply);
+
+ if (error == 0) {
+ conf->supply_enabled = true;
+ usleep_range(1000 * conf->post_power_on_delay_ms,
+ 1000 * (conf->post_power_on_delay_ms + 1));
+ }
+
+ return error;
+}
+
+static int spi_hid_of_assert_reset(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+
+ gpiod_set_value(conf->reset_gpio, 1);
+ return 0;
+}
+
+static int spi_hid_of_deassert_reset(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+
+ gpiod_set_value(conf->reset_gpio, 0);
+ return 0;
+}
+
+static void spi_hid_of_sleep_minimal_reset_delay(struct spihid_ops *ops)
+{
+ struct spi_hid_of_config *conf = container_of(ops,
+ struct spi_hid_of_config,
+ ops);
+ usleep_range(1000 * conf->minimal_reset_delay_ms,
+ 1000 * (conf->minimal_reset_delay_ms + 1));
+}
+
+static int spi_hid_of_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct spi_hid_of_config *config;
+ int error;
+
+ config = devm_kzalloc(dev, sizeof(struct spi_hid_of_config),
+ GFP_KERNEL);
+ if (!config)
+ return -ENOMEM;
+
+ config->ops.power_up = spi_hid_of_power_up;
+ config->ops.power_down = spi_hid_of_power_down;
+ config->ops.assert_reset = spi_hid_of_assert_reset;
+ config->ops.deassert_reset = spi_hid_of_deassert_reset;
+ config->ops.sleep_minimal_reset_delay =
+ spi_hid_of_sleep_minimal_reset_delay;
+
+ error = spi_hid_of_populate_config(config, dev);
+ if (error) {
+ dev_err(dev, "%s: unable to populate config data.", __func__);
+ return error;
+ }
+
+ return spi_hid_core_probe(spi, &config->ops, &config->property_conf);
+}
+
+const struct of_device_id spi_hid_of_match[] = {
+ { .compatible = "hid-over-spi" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, spi_hid_of_match);
+
+static const struct spi_device_id spi_hid_of_id_table[] = {
+ { "hid", 0 },
+ { "hid-over-spi", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, spi_hid_of_id_table);
+
+static struct spi_driver spi_hid_of_driver = {
+ .driver = {
+ .name = "spi_hid_of",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(spi_hid_of_match),
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .dev_groups = spi_hid_groups,
+ },
+ .probe = spi_hid_of_probe,
+ .remove = spi_hid_core_remove,
+ .id_table = spi_hid_of_id_table,
+};
+
+module_spi_driver(spi_hid_of_driver);
+
+MODULE_DESCRIPTION("HID over SPI OF transport driver");
+MODULE_AUTHOR("Dmitry Antipov <dmanti@microsoft.com>");
+MODULE_LICENSE("GPL");
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
` (7 preceding siblings ...)
2026-03-03 6:13 ` [PATCH 08/12] HID: spi_hid: add device tree " Jingyuan Liang
@ 2026-03-03 6:13 ` Jingyuan Liang
2026-03-03 7:24 ` Rob Herring (Arm)
` (2 more replies)
2026-03-03 6:13 ` [PATCH 10/12] HID: spi-hid: add power management implementation Jingyuan Liang
` (2 subsequent siblings)
11 siblings, 3 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang,
Dmitry Antipov, Jarrett Schultz
Documentation describes the required and optional properties for
implementing Device Tree for a Microsoft G6 Touch Digitizer that
supports HID over SPI Protocol 1.0 specification.
The properties are common to HID over SPI.
Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
.../devicetree/bindings/input/hid-over-spi.yaml | 153 +++++++++++++++++++++
1 file changed, 153 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/hid-over-spi.yaml b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
new file mode 100644
index 000000000000..b623629ed9d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
@@ -0,0 +1,153 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/hid-over-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HID over SPI Devices
+
+maintainers:
+ - Benjamin Tissoires <benjamin.tissoires@redhat.com>
+ - Jiri Kosina <jkosina@suse.cz>
+
+description: |+
+ HID over SPI provides support for various Human Interface Devices over the
+ SPI bus. These devices can be for example touchpads, keyboards, touch screens
+ or sensors.
+
+ The specification has been written by Microsoft and is currently available here:
+ https://www.microsoft.com/en-us/download/details.aspx?id=103325
+
+ If this binding is used, the kernel module spi-hid will handle the communication
+ with the device and the generic hid core layer will handle the protocol.
+
+allOf:
+ - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - microsoft,g6-touch-digitizer
+ - const: hid-over-spi
+ - description: Just "hid-over-spi" alone is allowed, but not recommended.
+ const: hid-over-spi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ GPIO specifier for the digitizer's reset pin (active low). The line must
+ be flagged with GPIO_ACTIVE_LOW.
+
+ vdd-supply:
+ description:
+ Regulator for the VDD supply voltage.
+
+ input-report-header-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 0xffffff
+ description:
+ A value to be included in the Read Approval packet, listing an address of
+ the input report header to be put on the SPI bus. This address has 24
+ bits.
+
+ input-report-body-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 0xffffff
+ description:
+ A value to be included in the Read Approval packet, listing an address of
+ the input report body to be put on the SPI bus. This address has 24 bits.
+
+ output-report-address:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 0xffffff
+ description:
+ A value to be included in the Output Report sent by the host, listing an
+ address where the output report on the SPI bus is to be written to. This
+ address has 24 bits.
+
+ post-power-on-delay-ms:
+ description:
+ Optional time in ms required by the device after enabling its regulators
+ or powering it on, before it is ready for communication.
+
+ minimal-reset-delay-ms:
+ description:
+ Optional minimum amount of time in ms that device needs to be in reset
+ state for the reset to take effect.
+
+ read-opcode:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description:
+ Value to be used in Read Approval packets. 1 byte.
+
+ write-opcode:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description:
+ Value to be used in Write Approval packets. 1 byte.
+
+ hid-over-spi-flags:
+ $ref: /schemas/types.yaml#/definitions/uint16
+ description:
+ 16 bits.
+ Bits 0-12 - Reserved (must be 0)
+ Bit 13 - SPI Write Mode. Possible values -
+ * 0b0- Writes are carried out in Single-SPI mode
+ * 0b1- Writes are carried out in the Multi-SPI mode specified by bits
+ 14-15
+ Bits 14-15 - Multi-SPI Mode. Possible values -
+ * 0b00- Single SPI
+ * 0b01- Dual SPI
+ * 0b10- Quad SPI
+
+required:
+ - compatible
+ - interrupts
+ - reset-gpios
+ - vdd-supply
+ - input-report-header-address
+ - input-report-body-address
+ - output-report-address
+ - read-opcode
+ - write-opcode
+ - hid-over-spi-flags
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hid@0 {
+ compatible = "hid-over-spi";
+ reg = <0x0>;
+ interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
+ vdd-supply = <&pm8350c_l3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
+ input-report-header-address = <0x1000>;
+ input-report-body-address = <0x1004>;
+ output-report-address = <0x2000>;
+ read-opcode = <0x0b>;
+ write-opcode = <0x02>;
+ hid-over-spi-flags = <0x0000>;
+ post-power-on-delay-ms = <5>;
+ minimal-reset-delay-ms = <5>;
+ };
+ };
\ No newline at end of file
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
2026-03-03 6:13 ` [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema Jingyuan Liang
@ 2026-03-03 7:24 ` Rob Herring (Arm)
2026-03-03 13:53 ` Rob Herring
2026-03-07 7:25 ` Val Packett
2 siblings, 0 replies; 24+ messages in thread
From: Rob Herring (Arm) @ 2026-03-03 7:24 UTC (permalink / raw)
To: Jingyuan Liang
Cc: Mark Brown, Benjamin Tissoires, Mathieu Desnoyers,
Jarrett Schultz, Dmitry Torokhov, Jonathan Corbet, Jiri Kosina,
linux-doc, linux-input, hbarnor, Steven Rostedt, Dmitry Antipov,
linux-spi, linux-trace-kernel, Krzysztof Kozlowski,
Masami Hiramatsu, devicetree, Conor Dooley, linux-kernel
On Tue, 03 Mar 2026 06:13:01 +0000, Jingyuan Liang wrote:
> Documentation describes the required and optional properties for
> implementing Device Tree for a Microsoft G6 Touch Digitizer that
> supports HID over SPI Protocol 1.0 specification.
>
> The properties are common to HID over SPI.
>
> Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
> Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> ---
> .../devicetree/bindings/input/hid-over-spi.yaml | 153 +++++++++++++++++++++
> 1 file changed, 153 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
./Documentation/devicetree/bindings/input/hid-over-spi.yaml:67:6: [warning] wrong indentation: expected 6 but found 5 (indentation)
./Documentation/devicetree/bindings/input/hid-over-spi.yaml:89:15: [error] empty value in block mapping (empty-values)
./Documentation/devicetree/bindings/input/hid-over-spi.yaml:153:7: [error] no new line character at the end of file (new-line-at-end-of-file)
./Documentation/devicetree/bindings/input/hid-over-spi.yaml:91:16: [error] syntax error: mapping values are not allowed here (syntax)
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/hid-over-spi.yaml: ignoring, error parsing file
./Documentation/devicetree/bindings/input/hid-over-spi.yaml:91:16: mapping values are not allowed here
make[2]: *** Deleting file 'Documentation/devicetree/bindings/input/hid-over-spi.example.dts'
Documentation/devicetree/bindings/input/hid-over-spi.yaml:91:16: mapping values are not allowed here
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/input/hid-over-spi.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1559: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260303-send-upstream-v1-9-1515ba218f3d@chromium.org
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
2026-03-03 6:13 ` [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema Jingyuan Liang
2026-03-03 7:24 ` Rob Herring (Arm)
@ 2026-03-03 13:53 ` Rob Herring
[not found] ` <CAEe3GZHSqepvjjopLwrWX3_n4+RnCeVVQnAO=Swixgu2z3OpUw@mail.gmail.com>
2026-03-07 7:25 ` Val Packett
2 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2026-03-03 13:53 UTC (permalink / raw)
To: Jingyuan Liang
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley, linux-input,
linux-doc, linux-kernel, linux-spi, linux-trace-kernel,
devicetree, hbarnor, Dmitry Antipov, Jarrett Schultz
On Tue, Mar 3, 2026 at 12:14 AM Jingyuan Liang <jingyliang@chromium.org> wrote:
>
> Documentation describes the required and optional properties for
> implementing Device Tree for a Microsoft G6 Touch Digitizer that
> supports HID over SPI Protocol 1.0 specification.
>
> The properties are common to HID over SPI.
>
> Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
> Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> ---
> .../devicetree/bindings/input/hid-over-spi.yaml | 153 +++++++++++++++++++++
> 1 file changed, 153 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/hid-over-spi.yaml b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> new file mode 100644
> index 000000000000..b623629ed9d3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> @@ -0,0 +1,153 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/hid-over-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HID over SPI Devices
> +
> +maintainers:
> + - Benjamin Tissoires <benjamin.tissoires@redhat.com>
> + - Jiri Kosina <jkosina@suse.cz>
> +
> +description: |+
> + HID over SPI provides support for various Human Interface Devices over the
> + SPI bus. These devices can be for example touchpads, keyboards, touch screens
> + or sensors.
> +
> + The specification has been written by Microsoft and is currently available here:
> + https://www.microsoft.com/en-us/download/details.aspx?id=103325
> +
> + If this binding is used, the kernel module spi-hid will handle the communication
> + with the device and the generic hid core layer will handle the protocol.
> +
> +allOf:
> + - $ref: /schemas/input/touchscreen/touchscreen.yaml#
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - enum:
> + - microsoft,g6-touch-digitizer
> + - const: hid-over-spi
> + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> + const: hid-over-spi
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + GPIO specifier for the digitizer's reset pin (active low). The line must
> + be flagged with GPIO_ACTIVE_LOW.
> +
> + vdd-supply:
> + description:
> + Regulator for the VDD supply voltage.
Is this part of the spec? This won't scale for multiple devices with
different power rails.
> +
> + input-report-header-address:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 0xffffff
> + description:
> + A value to be included in the Read Approval packet, listing an address of
> + the input report header to be put on the SPI bus. This address has 24
> + bits.
> +
> + input-report-body-address:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 0xffffff
> + description:
> + A value to be included in the Read Approval packet, listing an address of
> + the input report body to be put on the SPI bus. This address has 24 bits.
> +
> + output-report-address:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 0xffffff
> + description:
> + A value to be included in the Output Report sent by the host, listing an
> + address where the output report on the SPI bus is to be written to. This
> + address has 24 bits.
> +
> + post-power-on-delay-ms:
> + description:
> + Optional time in ms required by the device after enabling its regulators
> + or powering it on, before it is ready for communication.
Drop. This should be implied by the compatible.
> +
> + minimal-reset-delay-ms:
> + description:
> + Optional minimum amount of time in ms that device needs to be in reset
> + state for the reset to take effect.
Drop. This should be implied by the compatible.
> +
> + read-opcode:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description:
> + Value to be used in Read Approval packets. 1 byte.
> +
> + write-opcode:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description:
> + Value to be used in Write Approval packets. 1 byte.
Why are these and the address properties above not defined by the
spec? Do they vary for a specific device? If not, then they should be
implied by the compatible.
> +
> + hid-over-spi-flags:
> + $ref: /schemas/types.yaml#/definitions/uint16
> + description:
> + 16 bits.
> + Bits 0-12 - Reserved (must be 0)
> + Bit 13 - SPI Write Mode. Possible values -
> + * 0b0- Writes are carried out in Single-SPI mode
> + * 0b1- Writes are carried out in the Multi-SPI mode specified by bits
> + 14-15
> + Bits 14-15 - Multi-SPI Mode. Possible values -
> + * 0b00- Single SPI
> + * 0b01- Dual SPI
> + * 0b10- Quad SPI
We already have SPI properties to define the bus width for read and write.
> +
> +required:
> + - compatible
> + - interrupts
> + - reset-gpios
> + - vdd-supply
> + - input-report-header-address
> + - input-report-body-address
> + - output-report-address
> + - read-opcode
> + - write-opcode
> + - hid-over-spi-flags
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/gpio/gpio.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + hid@0 {
> + compatible = "hid-over-spi";
> + reg = <0x0>;
> + interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> + vdd-supply = <&pm8350c_l3>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
> + input-report-header-address = <0x1000>;
> + input-report-body-address = <0x1004>;
> + output-report-address = <0x2000>;
> + read-opcode = <0x0b>;
> + write-opcode = <0x02>;
> + hid-over-spi-flags = <0x0000>;
> + post-power-on-delay-ms = <5>;
> + minimal-reset-delay-ms = <5>;
> + };
> + };
> \ No newline at end of file
Fix this.
Rob
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
2026-03-03 6:13 ` [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema Jingyuan Liang
2026-03-03 7:24 ` Rob Herring (Arm)
2026-03-03 13:53 ` Rob Herring
@ 2026-03-07 7:25 ` Val Packett
2026-03-09 5:44 ` Dmitry Torokhov
2026-03-13 1:00 ` Jingyuan Liang
2 siblings, 2 replies; 24+ messages in thread
From: Val Packett @ 2026-03-07 7:25 UTC (permalink / raw)
To: Jingyuan Liang, Jiri Kosina, Benjamin Tissoires, Jonathan Corbet,
Mark Brown, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Dmitry Antipov,
Jarrett Schultz
On 3/3/26 3:13 AM, Jingyuan Liang wrote:
> Documentation describes the required and optional properties for
> implementing Device Tree for a Microsoft G6 Touch Digitizer that
> supports HID over SPI Protocol 1.0 specification.
> […]
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - enum:
> + - microsoft,g6-touch-digitizer
> + - const: hid-over-spi
> + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> […]
> +required:
> + - compatible
> + - interrupts
> + - reset-gpios
Why is reset required? Is it so implausible on some device implementing
the spec there wouldn't be a reset gpio?
> + - vdd-supply
Linux makes up a dummy regulator if DT doesn't provide one, so can
regulators even be required?
> […]
> + compatible = "hid-over-spi";
Not following your own recommendation from above :)
> + reg = <0x0>;
> + interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> + vdd-supply = <&pm8350c_l3>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
Heh, "reset_assert" is a name implying it would actually set the value
from the pinctrl properties, which is what had to be done before
reset-gpios were supported. But now reset-gpios are supported.
Thanks,
~val
P.S. happy to see work on this happen again!
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
2026-03-07 7:25 ` Val Packett
@ 2026-03-09 5:44 ` Dmitry Torokhov
2026-03-13 1:00 ` Jingyuan Liang
1 sibling, 0 replies; 24+ messages in thread
From: Dmitry Torokhov @ 2026-03-09 5:44 UTC (permalink / raw)
To: Val Packett
Cc: Jingyuan Liang, Jiri Kosina, Benjamin Tissoires, Jonathan Corbet,
Mark Brown, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
linux-doc, linux-kernel, linux-spi, linux-trace-kernel,
devicetree, hbarnor, Dmitry Antipov, Jarrett Schultz
On Sat, Mar 07, 2026 at 04:25:44AM -0300, Val Packett wrote:
>
> On 3/3/26 3:13 AM, Jingyuan Liang wrote:
> > Documentation describes the required and optional properties for
> > implementing Device Tree for a Microsoft G6 Touch Digitizer that
> > supports HID over SPI Protocol 1.0 specification.
> > […]
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - microsoft,g6-touch-digitizer
> > + - const: hid-over-spi
> > + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> > […]
> > +required:
> > + - compatible
> > + - interrupts
> > + - reset-gpios
>
> Why is reset required? Is it so implausible on some device implementing the
> spec there wouldn't be a reset gpio?
No, because it is mandated by the spec:
"HID SPI peripheral must provide a dedicated reset line, driven by the
HOST, which, when toggled (pulled LOW for at least 10ms, normally HIGH),
will have the effect of resetting the device. If a HID SPI peripheral is
enumerated via ACPI, the device ASL configuration must expose an ACPI
FLDR (_RST) method to control this line."
The spec also states that the host must initiate reset during
initialization of the device.
>
> > + - vdd-supply
> Linux makes up a dummy regulator if DT doesn't provide one, so can
> regulators even be required?
There is still a supply line to the chip even if it is not exposed to
the OS control. So as far as chip is concerned the supply is required.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
2026-03-07 7:25 ` Val Packett
2026-03-09 5:44 ` Dmitry Torokhov
@ 2026-03-13 1:00 ` Jingyuan Liang
1 sibling, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-13 1:00 UTC (permalink / raw)
To: Val Packett
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Dmitry Antipov,
Jarrett Schultz
On Fri, Mar 6, 2026 at 11:25 PM Val Packett <val@packett.cool> wrote:
>
>
> On 3/3/26 3:13 AM, Jingyuan Liang wrote:
> > Documentation describes the required and optional properties for
> > implementing Device Tree for a Microsoft G6 Touch Digitizer that
> > supports HID over SPI Protocol 1.0 specification.
> > […]
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - microsoft,g6-touch-digitizer
> > + - const: hid-over-spi
> > + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> > […]
> > +required:
> > + - compatible
> > + - interrupts
> > + - reset-gpios
>
> Why is reset required? Is it so implausible on some device implementing
> the spec there wouldn't be a reset gpio?
>
> > + - vdd-supply
> Linux makes up a dummy regulator if DT doesn't provide one, so can
> regulators even be required?
> > […]
> > + compatible = "hid-over-spi";
> Not following your own recommendation from above :)
Thanks! I will fix this in v2.
> > + reg = <0x0>;
> > + interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> > + reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> > + vdd-supply = <&pm8350c_l3>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
>
> Heh, "reset_assert" is a name implying it would actually set the value
> from the pinctrl properties, which is what had to be done before
> reset-gpios were supported. But now reset-gpios are supported.
Taken from the original patch. Will fix this in v2.
>
>
> Thanks,
> ~val
>
>
> P.S. happy to see work on this happen again!
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 10/12] HID: spi-hid: add power management implementation
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
` (8 preceding siblings ...)
2026-03-03 6:13 ` [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema Jingyuan Liang
@ 2026-03-03 6:13 ` Jingyuan Liang
2026-03-03 6:13 ` [PATCH 11/12] HID: spi-hid: add panel follower support Jingyuan Liang
2026-03-03 6:13 ` [PATCH 12/12] HID: spi-hid: add quirkis to support mode switch for Ilitek touch Jingyuan Liang
11 siblings, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang
Implement HID over SPI driver power management callbacks.
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-acpi.c | 1 +
drivers/hid/spi-hid/spi-hid-core.c | 107 +++++++++++++++++++++++++++++++++++++
drivers/hid/spi-hid/spi-hid-of.c | 1 +
drivers/hid/spi-hid/spi-hid.h | 1 +
4 files changed, 110 insertions(+)
diff --git a/drivers/hid/spi-hid/spi-hid-acpi.c b/drivers/hid/spi-hid/spi-hid-acpi.c
index 612e74fe72f9..2c1e4de99fea 100644
--- a/drivers/hid/spi-hid/spi-hid-acpi.c
+++ b/drivers/hid/spi-hid/spi-hid-acpi.c
@@ -238,6 +238,7 @@ static struct spi_driver spi_hid_acpi_driver = {
.driver = {
.name = "spi_hid_acpi",
.owner = THIS_MODULE,
+ .pm = &spi_hid_core_pm,
.acpi_match_table = ACPI_PTR(spi_hid_acpi_match),
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
.dev_groups = spi_hid_groups,
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 02beb209a92d..797ba99394f9 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -35,6 +35,8 @@
#include <linux/list.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/pm_wakeirq.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
#include <linux/string.h>
@@ -236,6 +238,81 @@ static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state
}
}
+static void spi_hid_suspend(struct spi_hid *shid)
+{
+ int error;
+ struct device *dev = &shid->spi->dev;
+
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_OFF)
+ return;
+
+ if (shid->hid) {
+ error = hid_driver_suspend(shid->hid, PMSG_SUSPEND);
+ if (error) {
+ dev_err(dev, "%s failed to suspend hid driver: %d",
+ __func__, error);
+ return;
+ }
+ }
+
+ disable_irq(shid->spi->irq);
+
+ clear_bit(SPI_HID_READY, &shid->flags);
+
+ if (!device_may_wakeup(dev)) {
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+
+ shid->ops->assert_reset(shid->ops);
+
+ error = shid->ops->power_down(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power down.", __func__);
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ return;
+ }
+
+ shid->power_state = HIDSPI_OFF;
+ }
+}
+
+static void spi_hid_resume(struct spi_hid *shid)
+{
+ int error;
+ struct device *dev = &shid->spi->dev;
+
+ guard(mutex)(&shid->power_lock);
+ if (shid->power_state == HIDSPI_ON)
+ return;
+
+ enable_irq(shid->spi->irq);
+
+ if (!device_may_wakeup(dev)) {
+ shid->ops->assert_reset(shid->ops);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ error = shid->ops->power_up(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power up.", __func__);
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ return;
+ }
+ shid->power_state = HIDSPI_ON;
+
+ shid->ops->deassert_reset(shid->ops);
+ }
+
+ if (shid->hid) {
+ error = hid_driver_reset_resume(shid->hid);
+ if (error)
+ dev_err(dev, "%s: failed to reset resume hid driver: %d.",
+ __func__, error);
+ }
+}
+
static void spi_hid_stop_hid(struct spi_hid *shid)
{
struct hid_device *hid = shid->hid;
@@ -1155,6 +1232,13 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
dev_err(dev, "%s: unable to request threaded IRQ.", __func__);
return error;
}
+ if (device_may_wakeup(dev)) {
+ error = dev_pm_set_wake_irq(dev, spi->irq);
+ if (error) {
+ dev_err(dev, "%s: failed to set wake IRQ.", __func__);
+ return error;
+ }
+ }
error = shid->ops->power_up(shid->ops);
if (error) {
@@ -1186,6 +1270,29 @@ void spi_hid_core_remove(struct spi_device *spi)
}
EXPORT_SYMBOL_GPL(spi_hid_core_remove);
+static int spi_hid_core_pm_suspend(struct device *dev)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ spi_hid_suspend(shid);
+
+ return 0;
+}
+
+static int spi_hid_core_pm_resume(struct device *dev)
+{
+ struct spi_hid *shid = dev_get_drvdata(dev);
+
+ spi_hid_resume(shid);
+
+ return 0;
+}
+
+const struct dev_pm_ops spi_hid_core_pm = {
+ SYSTEM_SLEEP_PM_OPS(spi_hid_core_pm_suspend, spi_hid_core_pm_resume)
+};
+EXPORT_SYMBOL_GPL(spi_hid_core_pm);
+
MODULE_DESCRIPTION("HID over SPI transport driver");
MODULE_AUTHOR("Dmitry Antipov <dmanti@microsoft.com>");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/spi-hid/spi-hid-of.c b/drivers/hid/spi-hid/spi-hid-of.c
index a20c8146230b..bc1d3c5a4dda 100644
--- a/drivers/hid/spi-hid/spi-hid-of.c
+++ b/drivers/hid/spi-hid/spi-hid-of.c
@@ -227,6 +227,7 @@ static struct spi_driver spi_hid_of_driver = {
.driver = {
.name = "spi_hid_of",
.owner = THIS_MODULE,
+ .pm = &spi_hid_core_pm,
.of_match_table = of_match_ptr(spi_hid_of_match),
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
.dev_groups = spi_hid_groups,
diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
index 1fdd45262647..5651c7fb706a 100644
--- a/drivers/hid/spi-hid/spi-hid.h
+++ b/drivers/hid/spi-hid/spi-hid.h
@@ -40,5 +40,6 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
void spi_hid_core_remove(struct spi_device *spi);
extern const struct attribute_group *spi_hid_groups[];
+extern const struct dev_pm_ops spi_hid_core_pm;
#endif /* SPI_HID_H */
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 11/12] HID: spi-hid: add panel follower support
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
` (9 preceding siblings ...)
2026-03-03 6:13 ` [PATCH 10/12] HID: spi-hid: add power management implementation Jingyuan Liang
@ 2026-03-03 6:13 ` Jingyuan Liang
2026-03-03 6:13 ` [PATCH 12/12] HID: spi-hid: add quirkis to support mode switch for Ilitek touch Jingyuan Liang
11 siblings, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang
Add support to spi-hid to be a panel follower.
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 199 +++++++++++++++++++++++++++++--------
drivers/hid/spi-hid/spi-hid-core.h | 7 ++
2 files changed, 163 insertions(+), 43 deletions(-)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 797ba99394f9..893a0d4642d2 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -238,21 +238,21 @@ static const char *spi_hid_power_mode_string(enum hidspi_power_state power_state
}
}
-static void spi_hid_suspend(struct spi_hid *shid)
+static int spi_hid_suspend(struct spi_hid *shid)
{
int error;
struct device *dev = &shid->spi->dev;
guard(mutex)(&shid->power_lock);
if (shid->power_state == HIDSPI_OFF)
- return;
+ return 0;
if (shid->hid) {
error = hid_driver_suspend(shid->hid, PMSG_SUSPEND);
if (error) {
dev_err(dev, "%s failed to suspend hid driver: %d",
__func__, error);
- return;
+ return error;
}
}
@@ -270,21 +270,22 @@ static void spi_hid_suspend(struct spi_hid *shid)
dev_err(dev, "%s: could not power down.", __func__);
shid->regulator_error_count++;
shid->regulator_last_error = error;
- return;
+ return error;
}
shid->power_state = HIDSPI_OFF;
}
+ return 0;
}
-static void spi_hid_resume(struct spi_hid *shid)
+static int spi_hid_resume(struct spi_hid *shid)
{
int error;
struct device *dev = &shid->spi->dev;
guard(mutex)(&shid->power_lock);
if (shid->power_state == HIDSPI_ON)
- return;
+ return 0;
enable_irq(shid->spi->irq);
@@ -298,7 +299,7 @@ static void spi_hid_resume(struct spi_hid *shid)
dev_err(dev, "%s: could not power up.", __func__);
shid->regulator_error_count++;
shid->regulator_last_error = error;
- return;
+ return error;
}
shid->power_state = HIDSPI_ON;
@@ -307,10 +308,13 @@ static void spi_hid_resume(struct spi_hid *shid)
if (shid->hid) {
error = hid_driver_reset_resume(shid->hid);
- if (error)
+ if (error) {
dev_err(dev, "%s: failed to reset resume hid driver: %d.",
__func__, error);
+ return error;
+ }
}
+ return 0;
}
static void spi_hid_stop_hid(struct spi_hid *shid)
@@ -1171,6 +1175,132 @@ const struct attribute_group *spi_hid_groups[] = {
};
EXPORT_SYMBOL_GPL(spi_hid_groups);
+/*
+ * At the end of probe we initialize the device:
+ * 0) assert reset, bias the interrupt line
+ * 1) sleep minimal reset delay
+ * 2) request IRQ
+ * 3) power up the device
+ * 4) deassert reset (high)
+ * After this we expect an IRQ with a reset response.
+ */
+static int spi_hid_dev_init(struct spi_hid *shid)
+{
+ struct spi_device *spi = shid->spi;
+ struct device *dev = &spi->dev;
+ int error;
+
+ shid->ops->assert_reset(shid->ops);
+
+ shid->ops->sleep_minimal_reset_delay(shid->ops);
+
+ error = devm_request_threaded_irq(dev, spi->irq, NULL, spi_hid_dev_irq,
+ IRQF_ONESHOT, dev_name(&spi->dev), shid);
+ if (error) {
+ dev_err(dev, "%s: unable to request threaded IRQ.", __func__);
+ return error;
+ }
+ if (device_may_wakeup(dev)) {
+ error = dev_pm_set_wake_irq(dev, spi->irq);
+ if (error) {
+ dev_err(dev, "%s: failed to set wake IRQ.", __func__);
+ return error;
+ }
+ }
+
+ error = shid->ops->power_up(shid->ops);
+ if (error) {
+ dev_err(dev, "%s: could not power up.", __func__);
+ shid->regulator_error_count++;
+ shid->regulator_last_error = error;
+ return error;
+ }
+
+ shid->ops->deassert_reset(shid->ops);
+
+ return 0;
+}
+
+static void spi_hid_panel_follower_work(struct work_struct *work)
+{
+ struct spi_hid *shid = container_of(work, struct spi_hid,
+ panel_follower_work);
+ int error;
+
+ if (!shid->desc.hid_version)
+ error = spi_hid_dev_init(shid);
+ else
+ error = spi_hid_resume(shid);
+ if (error)
+ dev_warn(&shid->spi->dev, "Power on failed: %d", error);
+ else
+ WRITE_ONCE(shid->panel_follower_work_finished, true);
+
+ /*
+ * The work APIs provide a number of memory ordering guarantees
+ * including one that says that memory writes before schedule_work()
+ * are always visible to the work function, but they don't appear to
+ * guarantee that a write that happened in the work is visible after
+ * cancel_work_sync(). We'll add a write memory barrier here to match
+ * with spi_hid_panel_unpreparing() to ensure that our write to
+ * panel_follower_work_finished is visible there.
+ */
+ smp_wmb();
+}
+
+static int spi_hid_panel_follower_resume(struct drm_panel_follower *follower)
+{
+ struct spi_hid *shid = container_of(follower, struct spi_hid, panel_follower);
+
+ /*
+ * Powering on a touchscreen can be a slow process. Queue the work to
+ * the system workqueue so we don't block the panel's power up.
+ */
+ WRITE_ONCE(shid->panel_follower_work_finished, false);
+ schedule_work(&shid->panel_follower_work);
+
+ return 0;
+}
+
+static int spi_hid_panel_follower_suspend(struct drm_panel_follower *follower)
+{
+ struct spi_hid *shid = container_of(follower, struct spi_hid, panel_follower);
+
+ cancel_work_sync(&shid->panel_follower_work);
+
+ /* Match with shid_core_panel_follower_work() */
+ smp_rmb();
+ if (!READ_ONCE(shid->panel_follower_work_finished))
+ return 0;
+
+ return spi_hid_suspend(shid);
+}
+
+static const struct drm_panel_follower_funcs
+ spi_hid_panel_follower_prepare_funcs = {
+ .panel_prepared = spi_hid_panel_follower_resume,
+ .panel_unpreparing = spi_hid_panel_follower_suspend,
+};
+
+static int spi_hid_register_panel_follower(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+
+ shid->panel_follower.funcs = &spi_hid_panel_follower_prepare_funcs;
+
+ /*
+ * If we're not in control of our own power up/power down then we can't
+ * do the logic to manage wakeups. Give a warning if a user thought
+ * that was possible then force the capability off.
+ */
+ if (device_can_wakeup(dev)) {
+ dev_warn(dev, "Can't wakeup if following panel\n");
+ device_set_wakeup_capable(dev, false);
+ }
+
+ return drm_panel_add_follower(dev, &shid->panel_follower);
+}
+
int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
struct spi_hid_conf *conf)
{
@@ -1190,6 +1320,7 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
shid->ops = ops;
shid->conf = conf;
set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ shid->is_panel_follower = drm_is_panel_follower(&spi->dev);
spi_set_drvdata(spi, shid);
@@ -1202,6 +1333,7 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
init_completion(&shid->output_done);
INIT_WORK(&shid->reset_work, spi_hid_reset_work);
+ INIT_WORK(&shid->panel_follower_work, spi_hid_panel_follower_work);
/*
* We need to allocate the buffer without knowing the maximum
@@ -1212,42 +1344,18 @@ int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
if (error)
return error;
- /*
- * At the end of probe we initialize the device:
- * 0) assert reset, bias the interrupt line
- * 1) sleep minimal reset delay
- * 2) request IRQ
- * 3) power up the device
- * 4) deassert reset (high)
- * After this we expect an IRQ with a reset response.
- */
-
- shid->ops->assert_reset(shid->ops);
-
- shid->ops->sleep_minimal_reset_delay(shid->ops);
-
- error = devm_request_threaded_irq(dev, spi->irq, NULL, spi_hid_dev_irq,
- IRQF_ONESHOT, dev_name(&spi->dev), shid);
- if (error) {
- dev_err(dev, "%s: unable to request threaded IRQ.", __func__);
- return error;
- }
- if (device_may_wakeup(dev)) {
- error = dev_pm_set_wake_irq(dev, spi->irq);
+ if (shid->is_panel_follower) {
+ error = spi_hid_register_panel_follower(shid);
if (error) {
- dev_err(dev, "%s: failed to set wake IRQ.", __func__);
+ dev_err(dev, "%s: could not add panel follower.", __func__);
return error;
}
+ } else {
+ error = spi_hid_dev_init(shid);
+ if (error)
+ return error;
}
- error = shid->ops->power_up(shid->ops);
- if (error) {
- dev_err(dev, "%s: could not power up.", __func__);
- return error;
- }
-
- shid->ops->deassert_reset(shid->ops);
-
dev_dbg(dev, "%s: d3 -> %s.", __func__,
spi_hid_power_mode_string(shid->power_state));
@@ -1261,6 +1369,9 @@ void spi_hid_core_remove(struct spi_device *spi)
struct device *dev = &spi->dev;
int error;
+ if (shid->is_panel_follower)
+ drm_panel_remove_follower(&shid->panel_follower);
+
spi_hid_stop_hid(shid);
shid->ops->assert_reset(shid->ops);
@@ -1274,18 +1385,20 @@ static int spi_hid_core_pm_suspend(struct device *dev)
{
struct spi_hid *shid = dev_get_drvdata(dev);
- spi_hid_suspend(shid);
+ if (shid->is_panel_follower)
+ return 0;
- return 0;
+ return spi_hid_suspend(shid);
}
static int spi_hid_core_pm_resume(struct device *dev)
{
struct spi_hid *shid = dev_get_drvdata(dev);
- spi_hid_resume(shid);
+ if (shid->is_panel_follower)
+ return 0;
- return 0;
+ return spi_hid_resume(shid);
}
const struct dev_pm_ops spi_hid_core_pm = {
diff --git a/drivers/hid/spi-hid/spi-hid-core.h b/drivers/hid/spi-hid/spi-hid-core.h
index 2bfdfbe6d7fc..88e9020d37aa 100644
--- a/drivers/hid/spi-hid/spi-hid-core.h
+++ b/drivers/hid/spi-hid/spi-hid-core.h
@@ -7,6 +7,8 @@
#include <linux/hid-over-spi.h>
#include <linux/spi/spi.h>
+#include <drm/drm_panel.h>
+
/* Protocol message size constants */
#define SPI_HID_READ_APPROVAL_LEN 5
#define SPI_HID_OUTPUT_HEADER_LEN 8
@@ -53,6 +55,10 @@ struct spi_hid {
struct spi_hid_input_buf *input; /* Input buffer. */
struct spi_hid_input_buf *response; /* Response buffer. */
+ struct drm_panel_follower panel_follower;
+ bool is_panel_follower;
+ bool panel_follower_work_finished;
+
u16 response_length;
u16 bufsize;
@@ -63,6 +69,7 @@ struct spi_hid {
unsigned long flags; /* device flags. */
struct work_struct reset_work;
+ struct work_struct panel_follower_work;
/* Control lock to make sure one output transaction at a time. */
struct mutex output_lock;
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 12/12] HID: spi-hid: add quirkis to support mode switch for Ilitek touch
2026-03-03 6:12 [PATCH 00/12] Add spi-hid transport driver Jingyuan Liang
` (10 preceding siblings ...)
2026-03-03 6:13 ` [PATCH 11/12] HID: spi-hid: add panel follower support Jingyuan Liang
@ 2026-03-03 6:13 ` Jingyuan Liang
11 siblings, 0 replies; 24+ messages in thread
From: Jingyuan Liang @ 2026-03-03 6:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Jingyuan Liang
Add quirks to support mode switch among Ilitek normal, debug and test mode
and allow delay before send output reports.
Add a shared variable to configure response timeout value for Ilitek
touch controllers.
Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
---
drivers/hid/spi-hid/spi-hid-core.c | 84 +++++++++++++++++++++++++++++++++++++-
drivers/hid/spi-hid/spi-hid-core.h | 4 ++
drivers/hid/spi-hid/spi-hid.h | 6 +++
3 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
index 893a0d4642d2..736e51f10cfc 100644
--- a/drivers/hid/spi-hid/spi-hid-core.c
+++ b/drivers/hid/spi-hid/spi-hid-core.c
@@ -22,6 +22,7 @@
#include <linux/completion.h>
#include <linux/crc32.h>
+#include <linux/delay.h>
#include <linux/device.h>
#include <linux/dma-mapping.h>
#include <linux/err.h>
@@ -45,9 +46,14 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include "../hid-ids.h"
#include "spi-hid.h"
#include "spi-hid-core.h"
+/* quirks to control the device */
+#define SPI_HID_QUIRK_MODE_SWITCH BIT(0)
+#define SPI_HID_QUIRK_READ_DELAY BIT(1)
+
/* Protocol constants */
#define SPI_HID_READ_APPROVAL_CONSTANT 0xff
#define SPI_HID_INPUT_HEADER_SYNC_BYTE 0x5a
@@ -86,6 +92,16 @@
#define SPI_HID_CREATE_DEVICE 4
#define SPI_HID_ERROR 5
+static const struct spi_hid_quirks {
+ __u16 idVendor;
+ __u16 idProduct;
+ __u32 quirks;
+} spi_hid_quirks[] = {
+ { USB_VENDOR_ID_ILITEK, HID_ANY_ID,
+ SPI_HID_QUIRK_MODE_SWITCH | SPI_HID_QUIRK_READ_DELAY },
+ { 0, 0 }
+};
+
/* Processed data from input report header */
struct spi_hid_input_header {
u8 version;
@@ -112,6 +128,27 @@ struct spi_hid_output_report {
static struct hid_ll_driver spi_hid_ll_driver;
+/**
+ * spi_hid_lookup_quirk: return any quirks associated with a SPI HID device
+ * @idVendor: the 16-bit vendor ID
+ * @idProduct: the 16-bit product ID
+ *
+ * Returns: a u32 quirks value.
+ */
+static u32 spi_hid_lookup_quirk(const u16 idVendor, const u16 idProduct)
+{
+ u32 quirks = 0;
+ int n;
+
+ for (n = 0; spi_hid_quirks[n].idVendor; n++)
+ if (spi_hid_quirks[n].idVendor == idVendor &&
+ (spi_hid_quirks[n].idProduct == (__u16)HID_ANY_ID ||
+ spi_hid_quirks[n].idProduct == idProduct))
+ quirks = spi_hid_quirks[n].quirks;
+
+ return quirks;
+}
+
static void spi_hid_populate_read_approvals(const struct spi_hid_conf *conf,
u8 *header_buf, u8 *body_buf)
{
@@ -382,6 +419,9 @@ static int spi_hid_send_output_report(struct spi_hid *shid,
u8 padding;
int error;
+ if (shid->quirks & SPI_HID_QUIRK_READ_DELAY)
+ usleep_range(2000, 2100);
+
guard(mutex)(&shid->output_lock);
if (report->content_length > shid->desc.max_output_length) {
dev_err(dev, "Output report too big, content_length 0x%x.",
@@ -406,18 +446,38 @@ static int spi_hid_send_output_report(struct spi_hid *shid,
return error;
}
+static const u32 spi_hid_get_timeout(struct spi_hid *shid)
+{
+ struct device *dev = &shid->spi->dev;
+ u32 timeout;
+
+ timeout = READ_ONCE(shid->ops->response_timeout_ms);
+
+ if (timeout < SPI_HID_RESP_TIMEOUT || timeout > 10000) {
+ dev_dbg(dev, "Response timeout is out of range, using default %d",
+ SPI_HID_RESP_TIMEOUT);
+ timeout = SPI_HID_RESP_TIMEOUT;
+ }
+
+ return timeout;
+}
+
static int spi_hid_sync_request(struct spi_hid *shid,
struct spi_hid_output_report *report)
{
struct device *dev = &shid->spi->dev;
+ u32 timeout = SPI_HID_RESP_TIMEOUT;
int error;
error = spi_hid_send_output_report(shid, report);
if (error)
return error;
+ if (shid->quirks & SPI_HID_QUIRK_MODE_SWITCH)
+ timeout = spi_hid_get_timeout(shid);
+
error = wait_for_completion_interruptible_timeout(&shid->output_done,
- msecs_to_jiffies(SPI_HID_RESP_TIMEOUT));
+ msecs_to_jiffies(timeout));
if (error == 0) {
dev_err(dev, "Response timed out.");
return -ETIMEDOUT;
@@ -561,6 +621,8 @@ static int spi_hid_create_device(struct spi_hid *shid)
hid->vendor = shid->desc.vendor_id;
hid->product = shid->desc.product_id;
+ shid->quirks = spi_hid_lookup_quirk(hid->vendor, hid->product);
+
snprintf(hid->name, sizeof(hid->name), "spi %04X:%04X",
hid->vendor, hid->product);
strscpy(hid->phys, dev_name(&shid->spi->dev), sizeof(hid->phys));
@@ -836,6 +898,24 @@ static irqreturn_t spi_hid_dev_irq(int irq, void *_shid)
goto out;
}
+ if (shid->quirks & SPI_HID_QUIRK_MODE_SWITCH) {
+ /*
+ * Update reset_pending on mode transitions inferred from
+ * response timeout (entering/exiting a mode).
+ */
+ u32 timeout = spi_hid_get_timeout(shid);
+ bool mode_enabled = timeout > SPI_HID_RESP_TIMEOUT;
+
+ if (mode_enabled != shid->prev_mode_enabled) {
+ if (mode_enabled)
+ set_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ else
+ clear_bit(SPI_HID_RESET_PENDING, &shid->flags);
+ }
+
+ shid->prev_mode_enabled = mode_enabled;
+ }
+
if (shid->input_message.status < 0) {
dev_warn(dev, "Error reading header: %d.",
shid->input_message.status);
@@ -1190,6 +1270,8 @@ static int spi_hid_dev_init(struct spi_hid *shid)
struct device *dev = &spi->dev;
int error;
+ shid->ops->custom_init(shid->ops);
+
shid->ops->assert_reset(shid->ops);
shid->ops->sleep_minimal_reset_delay(shid->ops);
diff --git a/drivers/hid/spi-hid/spi-hid-core.h b/drivers/hid/spi-hid/spi-hid-core.h
index 88e9020d37aa..8441dbad95d4 100644
--- a/drivers/hid/spi-hid/spi-hid-core.h
+++ b/drivers/hid/spi-hid/spi-hid-core.h
@@ -62,6 +62,10 @@ struct spi_hid {
u16 response_length;
u16 bufsize;
+ bool prev_mode_enabled; /* Previous device mode tracked for SPI_HID_QUIRK_MODE_SWITCH. */
+
+ unsigned long quirks; /* Various quirks. */
+
enum hidspi_power_state power_state;
u8 reset_attempts; /* The number of reset attempts. */
diff --git a/drivers/hid/spi-hid/spi-hid.h b/drivers/hid/spi-hid/spi-hid.h
index 5651c7fb706a..3c0369bdb4ab 100644
--- a/drivers/hid/spi-hid/spi-hid.h
+++ b/drivers/hid/spi-hid/spi-hid.h
@@ -25,6 +25,9 @@ struct spi_hid_conf {
* @power_down: do sequencing to power down the device
* @assert_reset: do sequencing to assert the reset line
* @deassert_reset: do sequencing to deassert the reset line
+ * @sleep_minimal_reset_delay: minimal sleep delay during reset
+ * @custom_init: customized device init
+ * @response_timeout_ms: output report response timeout in ms
*/
struct spihid_ops {
int (*power_up)(struct spihid_ops *ops);
@@ -32,6 +35,9 @@ struct spihid_ops {
int (*assert_reset)(struct spihid_ops *ops);
int (*deassert_reset)(struct spihid_ops *ops);
void (*sleep_minimal_reset_delay)(struct spihid_ops *ops);
+ int (*custom_init)(struct spihid_ops *ops);
+
+ u32 response_timeout_ms;
};
int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
--
2.53.0.473.g4a7958ca14-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread