* [PATCH v4 0/3] Input: Add TouchNetix axiom touchscreen driver
From: Kamel Bouhara @ 2023-12-04 14:05 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Marco Felsch, Jeff LaBundy
Cc: catalin.popescu, mark.satterthwaite, bartp, hannah.rossiter,
Thomas Petazzoni, Gregory Clement, bsp-development.geo,
Kamel Bouhara
Add a new driver for the TouchNetix's axiom family of
touchscreen controller. This driver only support i2c
and can be later adapted for SPI and USB support.
---
Changes in v2:
- Add device tree binding documentation
- Move core functions in axiom_i2c as we only care about i2c support now
- Use static function when required
- Use syntax dev_err_probe()
- Add an hardware based reset
Changes in v3:
- Remove irq-gpios property in dt-binding
- Use a generic node name
- Fix issues reported in https://lore.kernel.org/oe-kbuild-all/202310100300.oAC2M62R-lkp@intel.com/
Changes in v4:
- Cleanup unused headers and macros
- Use standard kernel type
- Namespace structures and functions
- Use packed struct when possible to avoid bitfield operators
- Fix missing break when address is found in axiom_populate_target_address()
- Split reads in two steps for the reports, first length then report
itself so we only read required bytes
- Get poll-interval from devicetree
- Add VDDI/VDDA regulators
- Add a startup delay of 110 ms required after VDDA/VDDI is applied
- Remove axiom_i2c_write() as it is no more used
Kamel Bouhara (3):
dt-bindings: vendor-prefixes: Add TouchNetix AS
dt-bindings: input: Add TouchNetix axiom touchscreen
Input: Add TouchNetix axiom i2c touchscreen driver
.../input/touchscreen/touchnetix,ax54a.yaml | 56 ++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 7 +
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/touchnetix_axiom.c | 675 ++++++++++++++++++
6 files changed, 753 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
create mode 100644 drivers/input/touchscreen/touchnetix_axiom.c
--
2.25.1
^ permalink raw reply
* [PATCH v4 1/3] dt-bindings: vendor-prefixes: Add TouchNetix AS
From: Kamel Bouhara @ 2023-12-04 14:05 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Marco Felsch, Jeff LaBundy
Cc: catalin.popescu, mark.satterthwaite, bartp, hannah.rossiter,
Thomas Petazzoni, Gregory Clement, bsp-development.geo,
Kamel Bouhara, Krzysztof Kozlowski
In-Reply-To: <20231204140505.2838916-1-kamel.bouhara@bootlin.com>
Add vendor prefix for TouchNetix AS (https://www.touchnetix.com/products/).
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 573578db9509..78581001a79c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1400,6 +1400,8 @@ patternProperties:
description: Toradex AG
"^toshiba,.*":
description: Toshiba Corporation
+ "^touchnetix,.*":
+ description: TouchNetix AS
"^toumaz,.*":
description: Toumaz
"^tpk,.*":
--
2.25.1
^ permalink raw reply related
* [PATCH v4 2/3] dt-bindings: input: Add TouchNetix axiom touchscreen
From: Kamel Bouhara @ 2023-12-04 14:05 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Marco Felsch, Jeff LaBundy
Cc: catalin.popescu, mark.satterthwaite, bartp, hannah.rossiter,
Thomas Petazzoni, Gregory Clement, bsp-development.geo,
Kamel Bouhara
In-Reply-To: <20231204140505.2838916-1-kamel.bouhara@bootlin.com>
Add the TouchNetix axiom I2C touchscreen device tree bindings
documentation.
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
.../input/touchscreen/touchnetix,ax54a.yaml | 56 +++++++++++++++++++
MAINTAINERS | 6 ++
2 files changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
new file mode 100644
index 000000000000..ff216a2bacc5
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/touchnetix,ax54a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TouchNetix Axiom series touchscreen controller
+
+maintainers:
+ - Kamel Bouhara <kamel.bouhara@bootlin.com>
+
+properties:
+ compatible:
+ const: touchnetix,ax54a
+
+ reg:
+ const: 0x66
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ poll-interval:
+ description: Poll interval time in milliseconds.
+
+ VDDA-supply:
+ description: Analog power supply regulator on VDDA pin
+
+ VDDI-supply:
+ description: I/O power supply regulator on VDDI pin
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ touchscreen@66 {
+ compatible = "touchnetix,ax54a";
+ reg = <0x66>;
+ interrupt-parent = <&gpio2>;
+ interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 7608b714653f..4752d8436dbb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21431,6 +21431,12 @@ S: Maintained
F: Documentation/ABI/testing/sysfs-class-firmware-attributes
F: drivers/platform/x86/think-lmi.?
+TOUCHNETIX AXIOM I2C TOUCHSCREEN DRIVER
+M: Kamel Bouhara <kamel.bouhara@bootlin.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
+
THUNDERBOLT DMA TRAFFIC TEST DRIVER
M: Isaac Hazan <isaac.hazan@intel.com>
L: linux-usb@vger.kernel.org
--
2.25.1
^ permalink raw reply related
* [PATCH v4 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Kamel Bouhara @ 2023-12-04 14:05 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
Marco Felsch, Jeff LaBundy
Cc: catalin.popescu, mark.satterthwaite, bartp, hannah.rossiter,
Thomas Petazzoni, Gregory Clement, bsp-development.geo,
Kamel Bouhara
In-Reply-To: <20231204140505.2838916-1-kamel.bouhara@bootlin.com>
Add a new driver for the TouchNetix's axiom family of
touchscreen controllers. This driver only supports i2c
and can be later adapted for SPI and USB support.
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
MAINTAINERS | 1 +
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/touchnetix_axiom.c | 675 +++++++++++++++++++
4 files changed, 689 insertions(+)
create mode 100644 drivers/input/touchscreen/touchnetix_axiom.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 4752d8436dbb..337ddac6c74b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21436,6 +21436,7 @@ M: Kamel Bouhara <kamel.bouhara@bootlin.com>
L: linux-input@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
+F: drivers/input/touchscreen/touchnetix_axiom.c
THUNDERBOLT DMA TRAFFIC TEST DRIVER
M: Isaac Hazan <isaac.hazan@intel.com>
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index e3e2324547b9..f36bee8d8696 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -803,6 +803,18 @@ config TOUCHSCREEN_MIGOR
To compile this driver as a module, choose M here: the
module will be called migor_ts.
+config TOUCHSCREEN_TOUCHNETIX_AXIOM
+ tristate "TouchNetix AXIOM based touchscreen controllers"
+ depends on I2C
+ help
+ Say Y here if you have a axiom touchscreen connected to
+ your system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called axiom.
+
config TOUCHSCREEN_TOUCHRIGHT
tristate "Touchright serial touchscreen"
select SERIO
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 62bd24f3ac8e..8e32a2df5e18 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -88,6 +88,7 @@ obj-$(CONFIG_TOUCHSCREEN_SUR40) += sur40.o
obj-$(CONFIG_TOUCHSCREEN_SURFACE3_SPI) += surface3_spi.o
obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC) += ti_am335x_tsc.o
obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213) += touchit213.o
+obj-$(CONFIG_TOUCHSCREEN_TOUCHNETIX_AXIOM) += touchnetix_axiom.o
obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT) += touchright.o
obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN) += touchwin.o
obj-$(CONFIG_TOUCHSCREEN_TS4800) += ts4800-ts.o
diff --git a/drivers/input/touchscreen/touchnetix_axiom.c b/drivers/input/touchscreen/touchnetix_axiom.c
new file mode 100644
index 000000000000..9783e89f9e3a
--- /dev/null
+++ b/drivers/input/touchscreen/touchnetix_axiom.c
@@ -0,0 +1,675 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * TouchNetix axiom Touchscreen Driver
+ *
+ * Copyright (C) 2020-2023 TouchNetix Ltd.
+ *
+ * Author(s): Bart Prescott <bartp@baasheep.co.uk>
+ * Pedro Torruella <pedro.torruella@touchnetix.com>
+ * Mark Satterthwaite <mark.satterthwaite@touchnetix.com>
+ * Hannah Rossiter <hannah.rossiter@touchnetix.com>
+ * Kamel Bouhara <kamel.bouhara@bootlin.com>
+ *
+ */
+#include <linux/bitfield.h>
+#include <linux/crc16.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#define AXIOM_PROX_LEVEL -128
+/*
+ * Register group u31 has 2 pages for usage table entries.
+ */
+#define AXIOM_U31_MAX_USAGES ((2 * AXIOM_COMMS_PAGE_SIZE) / AXIOM_U31_BYTES_PER_USAGE)
+#define AXIOM_U31_BYTES_PER_USAGE 6
+#define AXIOM_U31_PAGE0_LENGTH 0x0C
+#define AXIOM_U31_BOOTMODE_MASK BIT(7)
+#define AXIOM_U31_DEVID_MASK GENMASK(14, 0)
+
+#define AXIOM_CMD_HEADER_READ_MASK BIT(15)
+#define AXIOM_U41_MAX_TARGETS 10
+
+#define AXIOM_U46_AUX_CHANNELS 4
+#define AXIOM_U46_AUX_MASK GENMASK(11, 0)
+
+#define AXIOM_COMMS_MAX_USAGE_PAGES 3
+#define AXIOM_COMMS_PAGE_SIZE 256
+#define AXIOM_COMMS_REPORT_LEN_MASK GENMASK(6, 0)
+
+#define AXIOM_REPORT_USAGE_ID 0x34
+#define AXIOM_DEVINFO_USAGE_ID 0x31
+#define AXIOM_USAGE_2HB_REPORT_ID 0x01
+#define AXIOM_USAGE_2AUX_REPORT_ID 0x46
+#define AXIOM_USAGE_2DCTS_REPORT_ID 0x41
+
+#define AXIOM_PAGE_OFFSET_MASK GENMASK(6, 0)
+
+struct axiom_devinfo {
+ u16 device_id;
+ u8 fw_minor;
+ u8 fw_major;
+ u8 fw_info_extra;
+ u8 tcp_revision;
+ u8 bootloader_fw_minor;
+ u8 bootloader_fw_major;
+ u16 jedec_id;
+ u8 num_usages;
+} __packed;
+
+/*
+ * Describes parameters of a specific usage, essentially a single element of
+ * the "Usage Table"
+ */
+struct axiom_usage_entry {
+ u8 id;
+ u8 is_report;
+ u8 start_page;
+ u8 num_pages;
+};
+
+/*
+ * Represents state of a touch or target when detected prior a touch (eg.
+ * hover or proximity events).
+ */
+enum axiom_target_state {
+ AXIOM_TARGET_STATE_NOT_PRESENT = 0,
+ AXIOM_TARGET_STATE_PROX = 1,
+ AXIOM_TARGET_STATE_HOVER = 2,
+ AXIOM_TARGET_STATE_TOUCHING = 3,
+};
+
+struct axiom_u41_target {
+ enum axiom_target_state state;
+ u16 x;
+ u16 y;
+ s8 z;
+ bool insert;
+ bool touch;
+};
+
+struct axiom_target_report {
+ u8 index;
+ u8 present;
+ u16 x;
+ u16 y;
+ s8 z;
+};
+
+struct axiom_cmd_header {
+ __le16 target_address;
+ __le16 length;
+} __packed;
+
+struct axiom_data {
+ struct axiom_devinfo devinfo;
+ struct device *dev;
+ struct gpio_desc *reset_gpio;
+ struct i2c_client *client;
+ struct input_dev *input_dev;
+ u32 max_report_len;
+ char rx_buf[AXIOM_COMMS_MAX_USAGE_PAGES * AXIOM_COMMS_PAGE_SIZE];
+ struct axiom_u41_target targets[AXIOM_U41_MAX_TARGETS];
+ struct axiom_usage_entry usage_table[AXIOM_U31_MAX_USAGES];
+ bool usage_table_populated;
+ struct regulator *vdda;
+ struct regulator *vddi;
+};
+
+/*
+ * axiom devices are typically configured to report
+ * touches at a rate of 100Hz (10ms). For systems
+ * that require polling for reports.
+ * When reports are polled, it will be expected to
+ * occasionally observe the overflow bit being set
+ * in the reports. This indicates that reports are not
+ * being read fast enough.
+ */
+#define POLL_INTERVAL_DEFAULT_MS 10
+
+/* Translate usage/page/offset triplet into physical address. */
+static u16 axiom_usage_to_target_address(struct axiom_data *ts, char usage, char page,
+ char offset)
+{
+ struct axiom_usage_entry *usage_table;
+ struct axiom_devinfo *device_info;
+ u32 i;
+
+ device_info = &ts->devinfo;
+ usage_table = ts->usage_table;
+
+ /* At the moment the convention is that u31 is always at physical address 0x0 */
+ if (!ts->usage_table_populated) {
+ if (usage == AXIOM_DEVINFO_USAGE_ID)
+ return ((page << 8) + offset);
+ else
+ return 0xffff;
+ }
+
+ for (i = 0; i < ts->devinfo.num_usages; i++) {
+ if (usage_table[i].id != usage)
+ continue;
+
+ if (page >= usage_table[i].num_pages) {
+ dev_err(ts->dev, "Invalid usage table! usage: %u, page: %u, offset: %u\n",
+ usage, page, offset);
+ return 0xffff;
+ }
+ break;
+ }
+
+ return ((usage_table[i].start_page + page) << 8) + offset;
+}
+
+static int axiom_i2c_read(struct i2c_client *client, u8 usage, u8 page, u8 *buf, u16 len)
+{
+ struct axiom_data *ts = i2c_get_clientdata(client);
+ struct axiom_cmd_header cmd_header;
+ u8 buffer[sizeof(cmd_header) + AXIOM_COMMS_PAGE_SIZE];
+ struct i2c_msg msg;
+ int error;
+
+ cmd_header.target_address = cpu_to_le16(axiom_usage_to_target_address(ts, usage, page, 0));
+ cmd_header.length = cpu_to_le16(len | AXIOM_CMD_HEADER_READ_MASK);
+
+ memcpy(&buffer[0], &cmd_header, sizeof(cmd_header));
+ memcpy(&buffer[sizeof(cmd_header)], buf, len);
+
+ msg.addr = client->addr;
+ msg.flags = 0;
+ msg.len = len + sizeof(cmd_header);
+ msg.buf = (u8 *)buffer;
+
+ error = i2c_transfer(client->adapter, &msg, 1);
+ if (error != 1) {
+ dev_err(&client->dev,
+ "Failed reading usage %#x page %#x, error=%d\n",
+ usage, page, error);
+ return error;
+ }
+
+ usleep_range(100, 250);
+
+ return 0;
+}
+
+/*
+ * One of the main purposes for reading the usage table is to identify
+ * which usages reside at which TA address.
+ * When performing subsequent reads or writes to AXIOM, the TA is used
+ * to specify which usage is being accessed.
+ * Consider the following discovery code which will build up the usage table.
+ */
+static u32 axiom_populate_usage_table(struct axiom_data *ts)
+{
+ struct axiom_usage_entry *usage_table;
+ u32 max_report_len = 0;
+ char *rx_data = ts->rx_buf;
+ u32 usage_id;
+ int error;
+
+ usage_table = ts->usage_table;
+
+ /* Read the second page of usage u31 to get the usage table */
+ error = axiom_i2c_read(ts->client, AXIOM_DEVINFO_USAGE_ID, 1, rx_data,
+ (AXIOM_U31_BYTES_PER_USAGE * ts->devinfo.num_usages));
+ if (error)
+ return error;
+
+ for (usage_id = 0; usage_id < ts->devinfo.num_usages; usage_id++) {
+ u16 offset = (usage_id * AXIOM_U31_BYTES_PER_USAGE);
+ u8 id = rx_data[offset + 0];
+ u8 start_page = rx_data[offset + 1];
+ u8 num_pages = rx_data[offset + 2];
+ u32 max_offset = ((rx_data[offset + 3] & AXIOM_PAGE_OFFSET_MASK) + 1) * 2;
+
+ if (!num_pages)
+ usage_table[usage_id].is_report = true;
+
+ /* Store the entry into the usage table */
+ usage_table[usage_id].id = id;
+ usage_table[usage_id].start_page = start_page;
+ usage_table[usage_id].num_pages = num_pages;
+
+ dev_dbg(ts->dev, "Usage u%02x Info: %*ph\n", id,
+ AXIOM_U31_BYTES_PER_USAGE, &rx_data[offset]);
+
+ /* Identify the max report length the module will receive */
+ if (usage_table[usage_id].is_report && max_offset > max_report_len)
+ max_report_len = max_offset;
+ }
+
+ ts->usage_table_populated = true;
+
+ return max_report_len;
+}
+
+static int axiom_discover(struct axiom_data *ts)
+{
+ int error;
+
+ /*
+ * Fetch the first page of usage u31 to get the
+ * device information and the number of usages
+ */
+ error = axiom_i2c_read(ts->client, AXIOM_DEVINFO_USAGE_ID, 0, (char *)&ts->devinfo,
+ AXIOM_U31_PAGE0_LENGTH);
+ if (error)
+ return error;
+
+ dev_dbg(ts->dev, " Boot Mode : %s\n",
+ FIELD_GET(AXIOM_U31_BOOTMODE_MASK, ts->devinfo.device_id) ? "BLP" : "TCP");
+ dev_dbg(ts->dev, " Device ID : %04lx\n",
+ FIELD_GET(AXIOM_U31_DEVID_MASK, ts->devinfo.device_id));
+ dev_dbg(ts->dev, " Firmware Rev : %02x.%02x\n", ts->devinfo.fw_major,
+ ts->devinfo.fw_minor);
+ dev_dbg(ts->dev, " Bootloader Rev : %02x.%02x\n", ts->devinfo.bootloader_fw_major,
+ ts->devinfo.bootloader_fw_minor);
+ dev_dbg(ts->dev, " FW Extra Info : %04x\n", ts->devinfo.fw_info_extra);
+ dev_dbg(ts->dev, " Silicon : %04x\n", ts->devinfo.jedec_id);
+ dev_dbg(ts->dev, " Number usages : %04x\n", ts->devinfo.num_usages);
+
+ ts->max_report_len = axiom_populate_usage_table(ts);
+ if (!ts->max_report_len || !ts->devinfo.num_usages)
+ return -EINVAL;
+
+ dev_dbg(ts->dev, "Max Report Length: %u\n", ts->max_report_len);
+
+ return 0;
+}
+
+/*
+ * Support function to axiom_process_u41_report.
+ * Generates input-subsystem events for every target.
+ * After calling this function the caller shall issue
+ * a Sync to the input sub-system.
+ */
+static bool axiom_process_u41_report_target(struct axiom_data *ts,
+ struct axiom_target_report *target)
+{
+ struct input_dev *input_dev = ts->input_dev;
+ struct axiom_u41_target *target_prev_state;
+ enum axiom_target_state current_state;
+ bool update = false;
+ int slot;
+
+ /* Verify the target index */
+ if (target->index >= AXIOM_U41_MAX_TARGETS) {
+ dev_dbg(ts->dev, "Invalid target index! %u\n", target->index);
+ return false;
+ }
+
+ target_prev_state = &ts->targets[target->index];
+
+ current_state = AXIOM_TARGET_STATE_NOT_PRESENT;
+
+ if (target->present) {
+ if (target->z >= 0)
+ current_state = AXIOM_TARGET_STATE_TOUCHING;
+ else if (target->z > AXIOM_PROX_LEVEL && target->z < 0)
+ current_state = AXIOM_TARGET_STATE_HOVER;
+ else if (target->z == AXIOM_PROX_LEVEL)
+ current_state = AXIOM_TARGET_STATE_PROX;
+ }
+
+ if (target_prev_state->state == current_state &&
+ target_prev_state->x == target->x &&
+ target_prev_state->y == target->y &&
+ target_prev_state->z == target->z) {
+ return false;
+ }
+
+ slot = target->index;
+
+ dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
+ target->index, slot, target->present,
+ target->x, target->y, target->z);
+
+ switch (current_state) {
+ case AXIOM_TARGET_STATE_NOT_PRESENT:
+ case AXIOM_TARGET_STATE_PROX:
+ if (!target_prev_state->insert)
+ break;
+ update = true;
+ target_prev_state->insert = false;
+ input_mt_slot(input_dev, slot);
+
+ if (!slot)
+ input_report_key(input_dev, BTN_TOUCH, 0);
+
+ input_mt_report_slot_inactive(input_dev);
+ /*
+ * make sure the previous coordinates are
+ * all off screen when the finger comes back
+ */
+ target->x = 65535;
+ target->y = 65535;
+ target->z = AXIOM_PROX_LEVEL;
+ break;
+ case AXIOM_TARGET_STATE_HOVER:
+ case AXIOM_TARGET_STATE_TOUCHING:
+ target_prev_state->insert = true;
+ update = true;
+ input_mt_slot(input_dev, slot);
+ input_report_abs(input_dev, ABS_MT_TRACKING_ID, slot);
+ input_report_abs(input_dev, ABS_MT_POSITION_X, target->x);
+ input_report_abs(input_dev, ABS_X, target->x);
+ input_report_abs(input_dev, ABS_MT_POSITION_Y, target->y);
+ input_report_abs(input_dev, ABS_Y, target->y);
+
+ if (current_state == AXIOM_TARGET_STATE_TOUCHING) {
+ input_report_abs(input_dev, ABS_MT_DISTANCE, 0);
+ input_report_abs(input_dev, ABS_DISTANCE, 0);
+ input_report_abs(input_dev, ABS_MT_PRESSURE, target->z);
+ input_report_abs(input_dev, ABS_PRESSURE, target->z);
+ } else {
+ input_report_abs(input_dev, ABS_MT_DISTANCE, -target->z);
+ input_report_abs(input_dev, ABS_DISTANCE, -target->z);
+ input_report_abs(input_dev, ABS_MT_PRESSURE, 0);
+ input_report_abs(input_dev, ABS_PRESSURE, 0);
+ }
+
+ if (!slot)
+ input_report_key(input_dev, BTN_TOUCH, (current_state ==
+ AXIOM_TARGET_STATE_TOUCHING));
+ break;
+ default:
+ break;
+ }
+
+ target_prev_state->state = current_state;
+ target_prev_state->x = target->x;
+ target_prev_state->y = target->y;
+ target_prev_state->z = target->z;
+
+ return update;
+}
+
+/*
+ * U41 is the output report of the 2D CTS and contains the status of targets
+ * (including contacts and pre-contacts) along with their X,Y,Z values.
+ * When a target has been removed (no longer detected),
+ * the corresponding X,Y,Z values will be zeroed.
+ */
+static bool axiom_process_u41_report(struct axiom_data *ts, char *rx_buf)
+{
+ struct axiom_target_report target;
+ bool update_done = false;
+ u16 target_status;
+ u32 i;
+
+ target_status = ((rx_buf[1]) | (rx_buf[2] << 8));
+
+ for (i = 0; i < AXIOM_U41_MAX_TARGETS; i++) {
+ char target_step = i * 4;
+
+ target.index = i;
+ target.present = ((target_status & (1 << i)) != 0) ? 1 : 0;
+ target.x = (rx_buf[(target_step + 3)] | (rx_buf[target_step + 4] << 8));
+ target.y = (rx_buf[(target_step + 5)] | (rx_buf[target_step + 6] << 8));
+ target.z = (s8)(rx_buf[i + 43]);
+ update_done |= axiom_process_u41_report_target(ts, &target);
+ }
+
+ return update_done;
+}
+
+/*
+ * U46 report contains a low level measurement data generated by the CDS
+ * algorithms from the AUX channels. This information is useful when tuning
+ * multi-press to assess mechanical consistency in the unit's construction.
+ */
+static void axiom_process_u46_report(struct axiom_data *ts, char *rx_buf)
+{
+ struct input_dev *input_dev = ts->input_dev;
+ u32 event_value;
+ u16 aux_value;
+ u32 i = 0;
+
+ for (i = 0; i < AXIOM_U46_AUX_CHANNELS; i++) {
+ char target_step = i * 2;
+
+ aux_value = ((rx_buf[target_step + 2] << 8) | (rx_buf[target_step + 1]))
+ & AXIOM_U46_AUX_MASK;
+ event_value = (i << 16) | (aux_value);
+ input_event(input_dev, EV_MSC, MSC_RAW, event_value);
+ }
+}
+
+/*
+ * Validates the crc and demultiplexes the axiom reports to the appropriate
+ * report handler
+ */
+static int axiom_handle_events(struct axiom_data *ts)
+{
+ struct input_dev *input_dev = ts->input_dev;
+ char *report_data = ts->rx_buf;
+ char report_len;
+ struct device *dev = ts->dev;
+ u16 crc_report;
+ u16 crc_calc;
+ int error;
+
+ error = axiom_i2c_read(ts->client, AXIOM_REPORT_USAGE_ID, 0, &report_len,
+ sizeof(report_len));
+ if (error)
+ return error;
+
+ report_len = (report_len & AXIOM_COMMS_REPORT_LEN_MASK) << 1;
+ if (!report_len) {
+ dev_err(dev, "Zero length report discarded\n");
+ return -ENODATA;
+ }
+
+ error = axiom_i2c_read(ts->client, AXIOM_REPORT_USAGE_ID, 0, report_data, report_len);
+ if (error)
+ return error;
+
+ /* Validate the report CRC */
+ crc_report = (report_data[report_len - 1] << 8) | (report_data[report_len - 2]);
+ /* Length is in 16 bit words and remove the size of the CRC16 itself */
+ crc_calc = crc16(0, report_data, (report_len - 2));
+
+ if (crc_calc != crc_report) {
+ dev_err(dev,
+ "CRC mismatch! Expected: %#x, Calculated CRC: %#x.\n",
+ crc_report, crc_calc);
+ return -EINVAL;
+ }
+
+ switch (report_data[1]) {
+ case AXIOM_USAGE_2DCTS_REPORT_ID:
+ if (axiom_process_u41_report(ts, &report_data[1])) {
+ input_mt_sync_frame(input_dev);
+ input_sync(input_dev);
+ }
+ break;
+
+ case AXIOM_USAGE_2AUX_REPORT_ID:
+ /* This is an aux report (force) */
+ axiom_process_u46_report(ts, &report_data[1]);
+ input_mt_sync(input_dev);
+ input_sync(input_dev);
+ break;
+
+ case AXIOM_USAGE_2HB_REPORT_ID:
+ /* This is a heartbeat report */
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void axiom_i2c_poll(struct input_dev *input_dev)
+{
+ struct axiom_data *ts = input_get_drvdata(input_dev);
+
+ axiom_handle_events(ts);
+}
+
+static irqreturn_t axiom_irq(int irq, void *dev_id)
+{
+ struct axiom_data *ts = dev_id;
+
+ axiom_handle_events(ts);
+
+ return IRQ_HANDLED;
+}
+
+static void axiom_reset(struct gpio_desc *reset_gpio)
+{
+ gpiod_set_value_cansleep(reset_gpio, 1);
+ usleep_range(1000, 2000);
+ gpiod_set_value_cansleep(reset_gpio, 0);
+ msleep(110);
+}
+
+static int axiom_i2c_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct input_dev *input_dev;
+ struct axiom_data *ts;
+ u32 startup_delay_ms;
+ u32 poll_interval;
+ int target;
+ int error;
+
+ ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
+ ts->client = client;
+ i2c_set_clientdata(client, ts);
+ ts->dev = dev;
+
+ ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(ts->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
+
+ if (ts->reset_gpio)
+ axiom_reset(ts->reset_gpio);
+
+ ts->vddi = devm_regulator_get_optional(dev, "VDDI");
+ if (!IS_ERR(ts->vddi)) {
+ error = regulator_enable(ts->vddi);
+ if (error)
+ return dev_err_probe(&client->dev, PTR_ERR(ts->vddi),
+ "Failed to enable VDDI regulator\n");
+ }
+
+ ts->vdda = devm_regulator_get_optional(dev, "VDDA");
+ if (!IS_ERR(ts->vdda)) {
+ error = regulator_enable(ts->vdda);
+ if (error) {
+ dev_err(dev, "Failed to get VDDA regulator\n");
+ regulator_disable(ts->vddi);
+ return error;
+ }
+ if (!device_property_read_u32(dev, "startup-time-ms", &startup_delay_ms))
+ msleep(startup_delay_ms);
+ }
+
+ error = axiom_discover(ts);
+ if (error)
+ return dev_err_probe(dev, error, "Failed touchscreen discover\n");
+
+ input_dev = devm_input_allocate_device(ts->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ input_dev->name = "TouchNetix axiom Touchscreen";
+ input_dev->phys = "input/axiom_ts";
+
+ /* Single Touch */
+ input_set_abs_params(input_dev, ABS_X, 0, 65535, 0, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0, 65535, 0, 0);
+
+ /* Multi Touch */
+ /* Min, Max, Fuzz (expected noise in px, try 4?) and Flat */
+ input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, 65535, 0, 0);
+ /* Min, Max, Fuzz (expected noise in px, try 4?) and Flat */
+ input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, 65535, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
+
+ input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
+
+ /* Registers the axiom device as a touchscreen instead of a mouse pointer */
+ error = input_mt_init_slots(input_dev, AXIOM_U41_MAX_TARGETS, INPUT_MT_DIRECT);
+ if (error)
+ return error;
+
+ /* Enables the raw data for up to 4 force channels to be sent to the input subsystem */
+ set_bit(EV_REL, input_dev->evbit);
+ set_bit(EV_MSC, input_dev->evbit);
+ /* Declare that we support "RAW" Miscellaneous events */
+ set_bit(MSC_RAW, input_dev->mscbit);
+
+ ts->input_dev = input_dev;
+ input_set_drvdata(ts->input_dev, ts);
+
+ /* Ensure that all reports are initialised to not be present. */
+ for (target = 0; target < AXIOM_U41_MAX_TARGETS; target++)
+ ts->targets[target].state = AXIOM_TARGET_STATE_NOT_PRESENT;
+
+ error = input_register_device(input_dev);
+ if (error)
+ return dev_err_probe(ts->dev, error,
+ "Could not register with Input Sub-system.\n");
+
+ error = devm_request_threaded_irq(dev, client->irq, NULL,
+ axiom_irq, IRQF_ONESHOT, dev_name(dev), ts);
+ if (error < 0) {
+ dev_warn(dev, "Request irq failed, falling back to polling mode");
+
+ error = input_setup_polling(input_dev, axiom_i2c_poll);
+ if (error)
+ return dev_err_probe(ts->dev, error, "Unable to set up polling mode\n");
+
+ if (!device_property_read_u32(ts->dev, "poll-interval", &poll_interval))
+ input_set_poll_interval(input_dev, poll_interval);
+ else
+ input_set_poll_interval(input_dev, POLL_INTERVAL_DEFAULT_MS);
+ }
+
+ return 0;
+}
+
+static const struct i2c_device_id axiom_i2c_id_table[] = {
+ { "ax54a" },
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
+
+static const struct of_device_id axiom_i2c_of_match[] = {
+ { .compatible = "touchnetix,ax54a", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, axiom_i2c_of_match);
+
+static struct i2c_driver axiom_i2c_driver = {
+ .driver = {
+ .name = "axiom",
+ .of_match_table = axiom_i2c_of_match,
+ },
+ .id_table = axiom_i2c_id_table,
+ .probe = axiom_i2c_probe,
+};
+module_i2c_driver(axiom_i2c_driver);
+MODULE_AUTHOR("Bart Prescott <bartp@baasheep.co.uk>");
+MODULE_AUTHOR("Pedro Torruella <pedro.torruella@touchnetix.com>");
+MODULE_AUTHOR("Mark Satterthwaite <mark.satterthwaite@touchnetix.com>");
+MODULE_AUTHOR("Hannah Rossiter <hannah.rossiter@touchnetix.com>");
+MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
+MODULE_DESCRIPTION("TouchNetix axiom touchscreen I2C bus driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 2/2] pinctrl: amd: Mask non-wake source pins with interrupt enabled at suspend
From: Linus Walleij @ 2023-12-04 14:46 UTC (permalink / raw)
To: Mario Limonciello
Cc: Jiri Kosina, Benjamin Tissoires, open list:PIN CONTROL SUBSYSTEM,
open list, Basavaraj Natikar, open list:HID CORE LAYER,
Marcus Aram, Mark Herbert
In-Reply-To: <20231203032431.30277-3-mario.limonciello@amd.com>
On Sun, Dec 3, 2023 at 4:25 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
> If a pin isn't marked as a wake source processing any interrupts is
> just going to destroy battery life. The APU may wake up from a hardware
> sleep state to process the interrupt but not return control to the OS.
>
> Mask interrupt for all non-wake source pins at suspend. They'll be
> re-enabled at resume.
>
> Reported-and-tested-by: Marcus Aram <marcus+oss@oxar.nl>
> Reported-and-tested-by: Mark Herbert <mark.herbert42@gmail.com>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2812
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
I applied this patch (2/2) to the pin control tree for fixes.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v4 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: POPESCU Catalin @ 2023-12-04 15:09 UTC (permalink / raw)
To: Kamel Bouhara, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Marco Felsch, Jeff LaBundy
Cc: mark.satterthwaite@touchnetix.com, bartp@baasheep.co.uk,
hannah.rossiter@touchnetix.com, Thomas Petazzoni, Gregory Clement,
GEO-CHHER-bsp-development
In-Reply-To: <20231204140505.2838916-4-kamel.bouhara@bootlin.com>
On 04.12.23 15:05, Kamel Bouhara wrote:
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>
>
> Add a new driver for the TouchNetix's axiom family of
> touchscreen controllers. This driver only supports i2c
> and can be later adapted for SPI and USB support.
>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> ---
> MAINTAINERS | 1 +
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/touchnetix_axiom.c | 675 +++++++++++++++++++
> 4 files changed, 689 insertions(+)
> create mode 100644 drivers/input/touchscreen/touchnetix_axiom.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4752d8436dbb..337ddac6c74b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21436,6 +21436,7 @@ M: Kamel Bouhara <kamel.bouhara@bootlin.com>
> L: linux-input@vger.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
> +F: drivers/input/touchscreen/touchnetix_axiom.c
>
> THUNDERBOLT DMA TRAFFIC TEST DRIVER
> M: Isaac Hazan <isaac.hazan@intel.com>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index e3e2324547b9..f36bee8d8696 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -803,6 +803,18 @@ config TOUCHSCREEN_MIGOR
> To compile this driver as a module, choose M here: the
> module will be called migor_ts.
>
> +config TOUCHSCREEN_TOUCHNETIX_AXIOM
> + tristate "TouchNetix AXIOM based touchscreen controllers"
> + depends on I2C
> + help
> + Say Y here if you have a axiom touchscreen connected to
> + your system.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called axiom.
> +
> config TOUCHSCREEN_TOUCHRIGHT
> tristate "Touchright serial touchscreen"
> select SERIO
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 62bd24f3ac8e..8e32a2df5e18 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -88,6 +88,7 @@ obj-$(CONFIG_TOUCHSCREEN_SUR40) += sur40.o
> obj-$(CONFIG_TOUCHSCREEN_SURFACE3_SPI) += surface3_spi.o
> obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC) += ti_am335x_tsc.o
> obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213) += touchit213.o
> +obj-$(CONFIG_TOUCHSCREEN_TOUCHNETIX_AXIOM) += touchnetix_axiom.o
> obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT) += touchright.o
> obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN) += touchwin.o
> obj-$(CONFIG_TOUCHSCREEN_TS4800) += ts4800-ts.o
> diff --git a/drivers/input/touchscreen/touchnetix_axiom.c b/drivers/input/touchscreen/touchnetix_axiom.c
> new file mode 100644
> index 000000000000..9783e89f9e3a
> --- /dev/null
> +++ b/drivers/input/touchscreen/touchnetix_axiom.c
> @@ -0,0 +1,675 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * TouchNetix axiom Touchscreen Driver
> + *
> + * Copyright (C) 2020-2023 TouchNetix Ltd.
> + *
> + * Author(s): Bart Prescott <bartp@baasheep.co.uk>
> + * Pedro Torruella <pedro.torruella@touchnetix.com>
> + * Mark Satterthwaite <mark.satterthwaite@touchnetix.com>
> + * Hannah Rossiter <hannah.rossiter@touchnetix.com>
> + * Kamel Bouhara <kamel.bouhara@bootlin.com>
> + *
> + */
> +#include <linux/bitfield.h>
> +#include <linux/crc16.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#define AXIOM_PROX_LEVEL -128
> +/*
> + * Register group u31 has 2 pages for usage table entries.
> + */
> +#define AXIOM_U31_MAX_USAGES ((2 * AXIOM_COMMS_PAGE_SIZE) / AXIOM_U31_BYTES_PER_USAGE)
> +#define AXIOM_U31_BYTES_PER_USAGE 6
> +#define AXIOM_U31_PAGE0_LENGTH 0x0C
> +#define AXIOM_U31_BOOTMODE_MASK BIT(7)
> +#define AXIOM_U31_DEVID_MASK GENMASK(14, 0)
> +
> +#define AXIOM_CMD_HEADER_READ_MASK BIT(15)
> +#define AXIOM_U41_MAX_TARGETS 10
> +
> +#define AXIOM_U46_AUX_CHANNELS 4
> +#define AXIOM_U46_AUX_MASK GENMASK(11, 0)
> +
> +#define AXIOM_COMMS_MAX_USAGE_PAGES 3
> +#define AXIOM_COMMS_PAGE_SIZE 256
> +#define AXIOM_COMMS_REPORT_LEN_MASK GENMASK(6, 0)
> +
> +#define AXIOM_REPORT_USAGE_ID 0x34
> +#define AXIOM_DEVINFO_USAGE_ID 0x31
> +#define AXIOM_USAGE_2HB_REPORT_ID 0x01
> +#define AXIOM_USAGE_2AUX_REPORT_ID 0x46
> +#define AXIOM_USAGE_2DCTS_REPORT_ID 0x41
> +
> +#define AXIOM_PAGE_OFFSET_MASK GENMASK(6, 0)
> +
> +struct axiom_devinfo {
> + u16 device_id;
> + u8 fw_minor;
> + u8 fw_major;
> + u8 fw_info_extra;
> + u8 tcp_revision;
> + u8 bootloader_fw_minor;
> + u8 bootloader_fw_major;
> + u16 jedec_id;
> + u8 num_usages;
> +} __packed;
> +
> +/*
> + * Describes parameters of a specific usage, essentially a single element of
> + * the "Usage Table"
> + */
> +struct axiom_usage_entry {
> + u8 id;
> + u8 is_report;
> + u8 start_page;
> + u8 num_pages;
> +};
> +
> +/*
> + * Represents state of a touch or target when detected prior a touch (eg.
> + * hover or proximity events).
> + */
> +enum axiom_target_state {
> + AXIOM_TARGET_STATE_NOT_PRESENT = 0,
> + AXIOM_TARGET_STATE_PROX = 1,
> + AXIOM_TARGET_STATE_HOVER = 2,
> + AXIOM_TARGET_STATE_TOUCHING = 3,
> +};
> +
> +struct axiom_u41_target {
> + enum axiom_target_state state;
> + u16 x;
> + u16 y;
> + s8 z;
> + bool insert;
> + bool touch;
> +};
> +
> +struct axiom_target_report {
> + u8 index;
> + u8 present;
> + u16 x;
> + u16 y;
> + s8 z;
> +};
> +
> +struct axiom_cmd_header {
> + __le16 target_address;
> + __le16 length;
> +} __packed;
> +
> +struct axiom_data {
> + struct axiom_devinfo devinfo;
> + struct device *dev;
> + struct gpio_desc *reset_gpio;
> + struct i2c_client *client;
> + struct input_dev *input_dev;
> + u32 max_report_len;
> + char rx_buf[AXIOM_COMMS_MAX_USAGE_PAGES * AXIOM_COMMS_PAGE_SIZE];
> + struct axiom_u41_target targets[AXIOM_U41_MAX_TARGETS];
> + struct axiom_usage_entry usage_table[AXIOM_U31_MAX_USAGES];
> + bool usage_table_populated;
> + struct regulator *vdda;
> + struct regulator *vddi;
> +};
> +
> +/*
> + * axiom devices are typically configured to report
> + * touches at a rate of 100Hz (10ms). For systems
> + * that require polling for reports.
> + * When reports are polled, it will be expected to
> + * occasionally observe the overflow bit being set
> + * in the reports. This indicates that reports are not
> + * being read fast enough.
> + */
> +#define POLL_INTERVAL_DEFAULT_MS 10
> +
> +/* Translate usage/page/offset triplet into physical address. */
> +static u16 axiom_usage_to_target_address(struct axiom_data *ts, char usage, char page,
> + char offset)
> +{
> + struct axiom_usage_entry *usage_table;
> + struct axiom_devinfo *device_info;
> + u32 i;
> +
> + device_info = &ts->devinfo;
> + usage_table = ts->usage_table;
> +
> + /* At the moment the convention is that u31 is always at physical address 0x0 */
> + if (!ts->usage_table_populated) {
> + if (usage == AXIOM_DEVINFO_USAGE_ID)
> + return ((page << 8) + offset);
> + else
> + return 0xffff;
> + }
> +
> + for (i = 0; i < ts->devinfo.num_usages; i++) {
> + if (usage_table[i].id != usage)
> + continue;
> +
> + if (page >= usage_table[i].num_pages) {
> + dev_err(ts->dev, "Invalid usage table! usage: %u, page: %u, offset: %u\n",
> + usage, page, offset);
> + return 0xffff;
> + }
> + break;
> + }
> +
> + return ((usage_table[i].start_page + page) << 8) + offset;
> +}
> +
> +static int axiom_i2c_read(struct i2c_client *client, u8 usage, u8 page, u8 *buf, u16 len)
> +{
> + struct axiom_data *ts = i2c_get_clientdata(client);
> + struct axiom_cmd_header cmd_header;
> + u8 buffer[sizeof(cmd_header) + AXIOM_COMMS_PAGE_SIZE];
> + struct i2c_msg msg;
> + int error;
> +
> + cmd_header.target_address = cpu_to_le16(axiom_usage_to_target_address(ts, usage, page, 0));
> + cmd_header.length = cpu_to_le16(len | AXIOM_CMD_HEADER_READ_MASK);
> +
> + memcpy(&buffer[0], &cmd_header, sizeof(cmd_header));
> + memcpy(&buffer[sizeof(cmd_header)], buf, len);
You're copying a buffer that is meant to hold the response which makes
no sense.
Please stick to the original implementation which transfers 2 messages :
- first is the header (you're telling the controller what you want to
read) : this is a write and flags should be 0.
- second is the actual read message : flags should be I2C_M_RD
> +
> + msg.addr = client->addr;
> + msg.flags = 0;
> + msg.len = len + sizeof(cmd_header);
> + msg.buf = (u8 *)buffer;
> +
> + error = i2c_transfer(client->adapter, &msg, 1);
> + if (error != 1) {
You should transfer 2 messages (see above).
> + dev_err(&client->dev,
> + "Failed reading usage %#x page %#x, error=%d\n",
> + usage, page, error);
> + return error;
> + }
> +
> + usleep_range(100, 250);
The delay here is not enough as Touchnetix confirmed that we would need
250 usecs for the controller's internal DMA to be ready after an i2c
transfer.
> +
> + return 0;
> +}
> +
> +/*
> + * One of the main purposes for reading the usage table is to identify
> + * which usages reside at which TA address.
> + * When performing subsequent reads or writes to AXIOM, the TA is used
> + * to specify which usage is being accessed.
> + * Consider the following discovery code which will build up the usage table.
> + */
> +static u32 axiom_populate_usage_table(struct axiom_data *ts)
> +{
> + struct axiom_usage_entry *usage_table;
> + u32 max_report_len = 0;
> + char *rx_data = ts->rx_buf;
> + u32 usage_id;
> + int error;
> +
> + usage_table = ts->usage_table;
> +
> + /* Read the second page of usage u31 to get the usage table */
> + error = axiom_i2c_read(ts->client, AXIOM_DEVINFO_USAGE_ID, 1, rx_data,
> + (AXIOM_U31_BYTES_PER_USAGE * ts->devinfo.num_usages));
> + if (error)
> + return error;
> +
> + for (usage_id = 0; usage_id < ts->devinfo.num_usages; usage_id++) {
> + u16 offset = (usage_id * AXIOM_U31_BYTES_PER_USAGE);
> + u8 id = rx_data[offset + 0];
> + u8 start_page = rx_data[offset + 1];
> + u8 num_pages = rx_data[offset + 2];
> + u32 max_offset = ((rx_data[offset + 3] & AXIOM_PAGE_OFFSET_MASK) + 1) * 2;
> +
> + if (!num_pages)
> + usage_table[usage_id].is_report = true;
> +
> + /* Store the entry into the usage table */
> + usage_table[usage_id].id = id;
> + usage_table[usage_id].start_page = start_page;
> + usage_table[usage_id].num_pages = num_pages;
> +
> + dev_dbg(ts->dev, "Usage u%02x Info: %*ph\n", id,
> + AXIOM_U31_BYTES_PER_USAGE, &rx_data[offset]);
> +
> + /* Identify the max report length the module will receive */
> + if (usage_table[usage_id].is_report && max_offset > max_report_len)
> + max_report_len = max_offset;
> + }
> +
> + ts->usage_table_populated = true;
> +
> + return max_report_len;
> +}
> +
> +static int axiom_discover(struct axiom_data *ts)
> +{
> + int error;
> +
> + /*
> + * Fetch the first page of usage u31 to get the
> + * device information and the number of usages
> + */
> + error = axiom_i2c_read(ts->client, AXIOM_DEVINFO_USAGE_ID, 0, (char *)&ts->devinfo,
> + AXIOM_U31_PAGE0_LENGTH);
> + if (error)
> + return error;
> +
> + dev_dbg(ts->dev, " Boot Mode : %s\n",
> + FIELD_GET(AXIOM_U31_BOOTMODE_MASK, ts->devinfo.device_id) ? "BLP" : "TCP");
> + dev_dbg(ts->dev, " Device ID : %04lx\n",
> + FIELD_GET(AXIOM_U31_DEVID_MASK, ts->devinfo.device_id));
> + dev_dbg(ts->dev, " Firmware Rev : %02x.%02x\n", ts->devinfo.fw_major,
> + ts->devinfo.fw_minor);
> + dev_dbg(ts->dev, " Bootloader Rev : %02x.%02x\n", ts->devinfo.bootloader_fw_major,
> + ts->devinfo.bootloader_fw_minor);
> + dev_dbg(ts->dev, " FW Extra Info : %04x\n", ts->devinfo.fw_info_extra);
> + dev_dbg(ts->dev, " Silicon : %04x\n", ts->devinfo.jedec_id);
> + dev_dbg(ts->dev, " Number usages : %04x\n", ts->devinfo.num_usages);
> +
> + ts->max_report_len = axiom_populate_usage_table(ts);
> + if (!ts->max_report_len || !ts->devinfo.num_usages)
> + return -EINVAL;
> +
> + dev_dbg(ts->dev, "Max Report Length: %u\n", ts->max_report_len);
> +
> + return 0;
> +}
> +
> +/*
> + * Support function to axiom_process_u41_report.
> + * Generates input-subsystem events for every target.
> + * After calling this function the caller shall issue
> + * a Sync to the input sub-system.
> + */
> +static bool axiom_process_u41_report_target(struct axiom_data *ts,
> + struct axiom_target_report *target)
> +{
> + struct input_dev *input_dev = ts->input_dev;
> + struct axiom_u41_target *target_prev_state;
> + enum axiom_target_state current_state;
> + bool update = false;
> + int slot;
> +
> + /* Verify the target index */
> + if (target->index >= AXIOM_U41_MAX_TARGETS) {
> + dev_dbg(ts->dev, "Invalid target index! %u\n", target->index);
> + return false;
> + }
> +
> + target_prev_state = &ts->targets[target->index];
> +
> + current_state = AXIOM_TARGET_STATE_NOT_PRESENT;
> +
> + if (target->present) {
> + if (target->z >= 0)
> + current_state = AXIOM_TARGET_STATE_TOUCHING;
> + else if (target->z > AXIOM_PROX_LEVEL && target->z < 0)
> + current_state = AXIOM_TARGET_STATE_HOVER;
> + else if (target->z == AXIOM_PROX_LEVEL)
> + current_state = AXIOM_TARGET_STATE_PROX;
> + }
> +
> + if (target_prev_state->state == current_state &&
> + target_prev_state->x == target->x &&
> + target_prev_state->y == target->y &&
> + target_prev_state->z == target->z) {
> + return false;
> + }
> +
> + slot = target->index;
> +
> + dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
> + target->index, slot, target->present,
> + target->x, target->y, target->z);
> +
> + switch (current_state) {
> + case AXIOM_TARGET_STATE_NOT_PRESENT:
> + case AXIOM_TARGET_STATE_PROX:
> + if (!target_prev_state->insert)
> + break;
> + update = true;
> + target_prev_state->insert = false;
> + input_mt_slot(input_dev, slot);
> +
> + if (!slot)
> + input_report_key(input_dev, BTN_TOUCH, 0);
> +
> + input_mt_report_slot_inactive(input_dev);
> + /*
> + * make sure the previous coordinates are
> + * all off screen when the finger comes back
> + */
> + target->x = 65535;
> + target->y = 65535;
> + target->z = AXIOM_PROX_LEVEL;
> + break;
> + case AXIOM_TARGET_STATE_HOVER:
> + case AXIOM_TARGET_STATE_TOUCHING:
> + target_prev_state->insert = true;
> + update = true;
> + input_mt_slot(input_dev, slot);
> + input_report_abs(input_dev, ABS_MT_TRACKING_ID, slot);
> + input_report_abs(input_dev, ABS_MT_POSITION_X, target->x);
> + input_report_abs(input_dev, ABS_X, target->x);
> + input_report_abs(input_dev, ABS_MT_POSITION_Y, target->y);
> + input_report_abs(input_dev, ABS_Y, target->y);
> +
> + if (current_state == AXIOM_TARGET_STATE_TOUCHING) {
> + input_report_abs(input_dev, ABS_MT_DISTANCE, 0);
> + input_report_abs(input_dev, ABS_DISTANCE, 0);
> + input_report_abs(input_dev, ABS_MT_PRESSURE, target->z);
> + input_report_abs(input_dev, ABS_PRESSURE, target->z);
> + } else {
> + input_report_abs(input_dev, ABS_MT_DISTANCE, -target->z);
> + input_report_abs(input_dev, ABS_DISTANCE, -target->z);
> + input_report_abs(input_dev, ABS_MT_PRESSURE, 0);
> + input_report_abs(input_dev, ABS_PRESSURE, 0);
> + }
> +
> + if (!slot)
> + input_report_key(input_dev, BTN_TOUCH, (current_state ==
> + AXIOM_TARGET_STATE_TOUCHING));
> + break;
> + default:
> + break;
> + }
> +
> + target_prev_state->state = current_state;
> + target_prev_state->x = target->x;
> + target_prev_state->y = target->y;
> + target_prev_state->z = target->z;
> +
> + return update;
> +}
> +
> +/*
> + * U41 is the output report of the 2D CTS and contains the status of targets
> + * (including contacts and pre-contacts) along with their X,Y,Z values.
> + * When a target has been removed (no longer detected),
> + * the corresponding X,Y,Z values will be zeroed.
> + */
> +static bool axiom_process_u41_report(struct axiom_data *ts, char *rx_buf)
> +{
> + struct axiom_target_report target;
> + bool update_done = false;
> + u16 target_status;
> + u32 i;
> +
> + target_status = ((rx_buf[1]) | (rx_buf[2] << 8));
> +
> + for (i = 0; i < AXIOM_U41_MAX_TARGETS; i++) {
> + char target_step = i * 4;
> +
> + target.index = i;
> + target.present = ((target_status & (1 << i)) != 0) ? 1 : 0;
> + target.x = (rx_buf[(target_step + 3)] | (rx_buf[target_step + 4] << 8));
> + target.y = (rx_buf[(target_step + 5)] | (rx_buf[target_step + 6] << 8));
> + target.z = (s8)(rx_buf[i + 43]);
> + update_done |= axiom_process_u41_report_target(ts, &target);
> + }
> +
> + return update_done;
> +}
> +
> +/*
> + * U46 report contains a low level measurement data generated by the CDS
> + * algorithms from the AUX channels. This information is useful when tuning
> + * multi-press to assess mechanical consistency in the unit's construction.
> + */
> +static void axiom_process_u46_report(struct axiom_data *ts, char *rx_buf)
> +{
> + struct input_dev *input_dev = ts->input_dev;
> + u32 event_value;
> + u16 aux_value;
> + u32 i = 0;
> +
> + for (i = 0; i < AXIOM_U46_AUX_CHANNELS; i++) {
> + char target_step = i * 2;
> +
> + aux_value = ((rx_buf[target_step + 2] << 8) | (rx_buf[target_step + 1]))
> + & AXIOM_U46_AUX_MASK;
> + event_value = (i << 16) | (aux_value);
> + input_event(input_dev, EV_MSC, MSC_RAW, event_value);
> + }
> +}
> +
> +/*
> + * Validates the crc and demultiplexes the axiom reports to the appropriate
> + * report handler
> + */
> +static int axiom_handle_events(struct axiom_data *ts)
> +{
> + struct input_dev *input_dev = ts->input_dev;
> + char *report_data = ts->rx_buf;
> + char report_len;
> + struct device *dev = ts->dev;
> + u16 crc_report;
> + u16 crc_calc;
> + int error;
> +
> + error = axiom_i2c_read(ts->client, AXIOM_REPORT_USAGE_ID, 0, &report_len,
> + sizeof(report_len));
I know that I told you it would be better to do the read report in 2
phases, but we should have stick to Touchnetix recommendation that is to
read the whole report even if it's padded with zeroes.
This is to avoid the 250 usecs delay after the read of the report length.
> + if (error)
> + return error;
> +
> + report_len = (report_len & AXIOM_COMMS_REPORT_LEN_MASK) << 1;
> + if (!report_len) {
> + dev_err(dev, "Zero length report discarded\n");
> + return -ENODATA;
> + }
> +
> + error = axiom_i2c_read(ts->client, AXIOM_REPORT_USAGE_ID, 0, report_data, report_len);
> + if (error)
> + return error;
> +
> + /* Validate the report CRC */
> + crc_report = (report_data[report_len - 1] << 8) | (report_data[report_len - 2]);
> + /* Length is in 16 bit words and remove the size of the CRC16 itself */
> + crc_calc = crc16(0, report_data, (report_len - 2));
> +
> + if (crc_calc != crc_report) {
> + dev_err(dev,
> + "CRC mismatch! Expected: %#x, Calculated CRC: %#x.\n",
> + crc_report, crc_calc);
> + return -EINVAL;
> + }
> +
> + switch (report_data[1]) {
> + case AXIOM_USAGE_2DCTS_REPORT_ID:
> + if (axiom_process_u41_report(ts, &report_data[1])) {
> + input_mt_sync_frame(input_dev);
> + input_sync(input_dev);
> + }
> + break;
> +
> + case AXIOM_USAGE_2AUX_REPORT_ID:
> + /* This is an aux report (force) */
> + axiom_process_u46_report(ts, &report_data[1]);
> + input_mt_sync(input_dev);
> + input_sync(input_dev);
> + break;
> +
> + case AXIOM_USAGE_2HB_REPORT_ID:
> + /* This is a heartbeat report */
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static void axiom_i2c_poll(struct input_dev *input_dev)
> +{
> + struct axiom_data *ts = input_get_drvdata(input_dev);
> +
> + axiom_handle_events(ts);
> +}
> +
> +static irqreturn_t axiom_irq(int irq, void *dev_id)
> +{
> + struct axiom_data *ts = dev_id;
> +
> + axiom_handle_events(ts);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void axiom_reset(struct gpio_desc *reset_gpio)
> +{
> + gpiod_set_value_cansleep(reset_gpio, 1);
> + usleep_range(1000, 2000);
> + gpiod_set_value_cansleep(reset_gpio, 0);
> + msleep(110);
> +}
> +
> +static int axiom_i2c_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct input_dev *input_dev;
> + struct axiom_data *ts;
> + u32 startup_delay_ms;
> + u32 poll_interval;
> + int target;
> + int error;
> +
> + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + ts->client = client;
> + i2c_set_clientdata(client, ts);
> + ts->dev = dev;
> +
> + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(ts->reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
> +
> + if (ts->reset_gpio)
> + axiom_reset(ts->reset_gpio);
> +
> + ts->vddi = devm_regulator_get_optional(dev, "VDDI");
> + if (!IS_ERR(ts->vddi)) {
> + error = regulator_enable(ts->vddi);
> + if (error)
> + return dev_err_probe(&client->dev, PTR_ERR(ts->vddi),
> + "Failed to enable VDDI regulator\n");
> + }
> +
> + ts->vdda = devm_regulator_get_optional(dev, "VDDA");
> + if (!IS_ERR(ts->vdda)) {
> + error = regulator_enable(ts->vdda);
> + if (error) {
> + dev_err(dev, "Failed to get VDDA regulator\n");
> + regulator_disable(ts->vddi);
> + return error;
> + }
> + if (!device_property_read_u32(dev, "startup-time-ms", &startup_delay_ms))
> + msleep(startup_delay_ms);
> + }
> +
> + error = axiom_discover(ts);
> + if (error)
> + return dev_err_probe(dev, error, "Failed touchscreen discover\n");
> +
> + input_dev = devm_input_allocate_device(ts->dev);
> + if (!input_dev)
> + return -ENOMEM;
> +
> + input_dev->name = "TouchNetix axiom Touchscreen";
> + input_dev->phys = "input/axiom_ts";
> +
> + /* Single Touch */
> + input_set_abs_params(input_dev, ABS_X, 0, 65535, 0, 0);
> + input_set_abs_params(input_dev, ABS_Y, 0, 65535, 0, 0);
> +
> + /* Multi Touch */
> + /* Min, Max, Fuzz (expected noise in px, try 4?) and Flat */
> + input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, 65535, 0, 0);
> + /* Min, Max, Fuzz (expected noise in px, try 4?) and Flat */
> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, 65535, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
> +
> + input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
> +
> + /* Registers the axiom device as a touchscreen instead of a mouse pointer */
> + error = input_mt_init_slots(input_dev, AXIOM_U41_MAX_TARGETS, INPUT_MT_DIRECT);
> + if (error)
> + return error;
> +
> + /* Enables the raw data for up to 4 force channels to be sent to the input subsystem */
> + set_bit(EV_REL, input_dev->evbit);
> + set_bit(EV_MSC, input_dev->evbit);
> + /* Declare that we support "RAW" Miscellaneous events */
> + set_bit(MSC_RAW, input_dev->mscbit);
> +
> + ts->input_dev = input_dev;
> + input_set_drvdata(ts->input_dev, ts);
> +
> + /* Ensure that all reports are initialised to not be present. */
> + for (target = 0; target < AXIOM_U41_MAX_TARGETS; target++)
> + ts->targets[target].state = AXIOM_TARGET_STATE_NOT_PRESENT;
> +
> + error = input_register_device(input_dev);
> + if (error)
> + return dev_err_probe(ts->dev, error,
> + "Could not register with Input Sub-system.\n");
> +
> + error = devm_request_threaded_irq(dev, client->irq, NULL,
> + axiom_irq, IRQF_ONESHOT, dev_name(dev), ts);
> + if (error < 0) {
> + dev_warn(dev, "Request irq failed, falling back to polling mode");
> +
> + error = input_setup_polling(input_dev, axiom_i2c_poll);
> + if (error)
> + return dev_err_probe(ts->dev, error, "Unable to set up polling mode\n");
> +
> + if (!device_property_read_u32(ts->dev, "poll-interval", &poll_interval))
> + input_set_poll_interval(input_dev, poll_interval);
> + else
> + input_set_poll_interval(input_dev, POLL_INTERVAL_DEFAULT_MS);
> + }
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id axiom_i2c_id_table[] = {
> + { "ax54a" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
> +
> +static const struct of_device_id axiom_i2c_of_match[] = {
> + { .compatible = "touchnetix,ax54a", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, axiom_i2c_of_match);
> +
> +static struct i2c_driver axiom_i2c_driver = {
> + .driver = {
> + .name = "axiom",
> + .of_match_table = axiom_i2c_of_match,
> + },
> + .id_table = axiom_i2c_id_table,
> + .probe = axiom_i2c_probe,
> +};
> +module_i2c_driver(axiom_i2c_driver);
> +MODULE_AUTHOR("Bart Prescott <bartp@baasheep.co.uk>");
> +MODULE_AUTHOR("Pedro Torruella <pedro.torruella@touchnetix.com>");
> +MODULE_AUTHOR("Mark Satterthwaite <mark.satterthwaite@touchnetix.com>");
> +MODULE_AUTHOR("Hannah Rossiter <hannah.rossiter@touchnetix.com>");
> +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
> +MODULE_DESCRIPTION("TouchNetix axiom touchscreen I2C bus driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH 1/2] HID: i2c-hid: Add IDEA5002 to i2c_hid_acpi_blacklist[]
From: Jiri Kosina @ 2023-12-04 15:18 UTC (permalink / raw)
To: Mario Limonciello
Cc: Linus Walleij, Benjamin Tissoires,
open list:PIN CONTROL SUBSYSTEM, open list, Basavaraj Natikar,
open list:HID CORE LAYER, stable, Marcus Aram, Mark Herbert
In-Reply-To: <20231203032431.30277-2-mario.limonciello@amd.com>
On Sat, 2 Dec 2023, Mario Limonciello wrote:
> Users have reported problems with recent Lenovo laptops that contain
> an IDEA5002 I2C HID device. Reports include fans turning on and
> running even at idle and spurious wakeups from suspend.
>
> Presumably in the Windows ecosystem there is an application that
> uses the HID device. Maybe that puts it into a lower power state so
> it doesn't cause spurious events.
>
> This device doesn't serve any functional purpose in Linux as nothing
> interacts with it so blacklist it from being probed. This will
> prevent the GPIO driver from setting up the GPIO and the spurious
> interrupts and wake events will not occur.
>
> Cc: stable@vger.kernel.org # 6.1
> Reported-and-tested-by: Marcus Aram <marcus+oss@oxar.nl>
> Reported-and-tested-by: Mark Herbert <mark.herbert42@gmail.com>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2812
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/hid/i2c-hid/i2c-hid-acpi.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> index ac918a9ea8d3..1b49243adb16 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
> @@ -40,6 +40,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
> * ICN8505 controller, has a _CID of PNP0C50 but is not HID compatible.
> */
> { "CHPN0001" },
> + /*
> + * The IDEA5002 ACPI device causes high interrupt usage and spurious
> + * wakeups from suspend.
> + */
> + { "IDEA5002" },
Applied to hid.git#for-6.7/upstream-fixes. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: Add quirk for Labtec/ODDOR/aikeec handbrake
From: Jiri Kosina @ 2023-12-04 15:21 UTC (permalink / raw)
To: Sebastian Parschauer; +Cc: Benjamin Tissoires, linux-input
In-Reply-To: <20231127224937.9407-1-s.parschauer@gmx.de>
On Mon, 27 Nov 2023, Sebastian Parschauer wrote:
> This device needs ALWAYS_POLL quirk, otherwise it keeps reconnecting
> indefinitely. It is a handbrake for sim racing detected as joystick.
> Reported and tested by GitHub user N0th1ngM4tt3rs.
>
> Link: https://github.com/sriemer/fix-linux-mouse issue 22
> Signed-off-by: Sebastian Parschauer <s.parschauer@gmx.de>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH v3 2/5] i2c: of: Introduce component probe function
From: Doug Anderson @ 2023-12-04 16:03 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Wolfram Sang, Rob Herring, Frank Rowand, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAGXv+5EpA531O1tW=h1RvK34+LMvtdve3=cNmfN=2+9t1jL_bA@mail.gmail.com>
Hi,
On Mon, Dec 4, 2023 at 1:53 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> > IMO you should prototype how you're going to handle regulators and
> > GPIOs before finalizing the design. I was going to write that you
> > should just document that it was up to the caller to power things up
> > before calling this function, but then I realized that the caller
> > would have to duplicate much of this function in order to do so. In
> > the very least they'd have to find the nodes of the relevant devices
> > so that they could grab regulators and/or GPIOs. In order to avoid
> > this duplication, would the design need to change? Perhaps this would
> > be as simple as adding a callback function here that's called with all
> > of the nodes before probing? If that's right, it would be nice to have
> > that callback from the beginning so we don't need two variants of the
> > function...
>
> So I think I can prototype designs with one GPIO and multiple regulators,
> assuming each node has the same number of both? At least they should if
> they're on the same connector.
>
> More than one GPIO probably means there are some ordering and timing
> constraints, and won't be as generic.
I was hoping to see a prototype of how this could work in the
non-generic case where the board needed a custom function to power
things up. It seems like we'd still want to be able to use your code
for probing.
> > > + for_each_child_of_node(i2c_node, node) {
> > > + union i2c_smbus_data data;
> > > + u32 addr;
> > > +
> > > + if (!of_node_name_prefix(node, type))
> > > + continue;
> > > + if (of_property_read_u32(node, "reg", &addr))
> > > + continue;
> > > + if (i2c_smbus_xfer(i2c, addr, 0, I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data) < 0)
> >
> > I'd be tempted to say that the caller should be able to pass in a
> > function pointer here so they could use an alternative method to probe
> > instead of i2c_smbus_xfer(), though you'd want to make it easy to
> > default to i2c_smbus_xfer(). I could imagine someone might need a
> > different way to probe. For instance if you had two touchscreens both
> > at the same "reg" but that had different "hid-descr-addr" then this
> > could be important.
>
> I'd say the only specific probable type is hid-i2c. And that could be
> generic enough that we could incorporate it here if we wanted. However
> I think we want to keep the initial version a bit simpler.
I don't mind if the initial version is simpler, but I'd love to
understand how this will grow. It doesn't feel terrible to take in a
function pointer that will probe the device and then provide a
function that callers could pass in that simply did the simple
i2c_smbus_xfer().
> > > + continue;
> > > +
> >
> > Put the "break" right here. You've found the device and that was the
> > point of the loop.
>
> In its place we'd have an if (node) { <enable node> } block. I guess it
> makes it easier to read still?
...or perhaps an "if (!node) goto exit" block and then you don't need
indentation? Essentially the loop becomes the implementation: "node =
find_the_one_that_exists(...)".
-Doug
^ permalink raw reply
* Re: [PATCH v4 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Kamel Bouhara @ 2023-12-04 16:16 UTC (permalink / raw)
To: POPESCU Catalin
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Marco Felsch, Jeff LaBundy, mark.satterthwaite@touchnetix.com,
bartp@baasheep.co.uk, hannah.rossiter@touchnetix.com,
Thomas Petazzoni, Gregory Clement, GEO-CHHER-bsp-development
In-Reply-To: <76a14af9-dc51-4d87-9110-a8be05702b56@leica-geosystems.com>
Le Mon, Dec 04, 2023 at 03:09:58PM +0000, POPESCU Catalin a écrit :
> On 04.12.23 15:05, Kamel Bouhara wrote:
> > This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
> >
> >
> > Add a new driver for the TouchNetix's axiom family of
> > touchscreen controllers. This driver only supports i2c
> > and can be later adapted for SPI and USB support.
> >
> > Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/input/touchscreen/Kconfig | 12 +
> > drivers/input/touchscreen/Makefile | 1 +
> > drivers/input/touchscreen/touchnetix_axiom.c | 675 +++++++++++++++++++
> > 4 files changed, 689 insertions(+)
> > create mode 100644 drivers/input/touchscreen/touchnetix_axiom.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 4752d8436dbb..337ddac6c74b 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -21436,6 +21436,7 @@ M: Kamel Bouhara <kamel.bouhara@bootlin.com>
> > L: linux-input@vger.kernel.org
> > S: Maintained
> > F: Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
> > +F: drivers/input/touchscreen/touchnetix_axiom.c
> >
> > THUNDERBOLT DMA TRAFFIC TEST DRIVER
> > M: Isaac Hazan <isaac.hazan@intel.com>
> > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> > index e3e2324547b9..f36bee8d8696 100644
> > --- a/drivers/input/touchscreen/Kconfig
> > +++ b/drivers/input/touchscreen/Kconfig
> > @@ -803,6 +803,18 @@ config TOUCHSCREEN_MIGOR
> > To compile this driver as a module, choose M here: the
> > module will be called migor_ts.
> >
> > +config TOUCHSCREEN_TOUCHNETIX_AXIOM
> > + tristate "TouchNetix AXIOM based touchscreen controllers"
> > + depends on I2C
> > + help
> > + Say Y here if you have a axiom touchscreen connected to
> > + your system.
> > +
> > + If unsure, say N.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called axiom.
> > +
> > config TOUCHSCREEN_TOUCHRIGHT
> > tristate "Touchright serial touchscreen"
> > select SERIO
> > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> > index 62bd24f3ac8e..8e32a2df5e18 100644
> > --- a/drivers/input/touchscreen/Makefile
> > +++ b/drivers/input/touchscreen/Makefile
> > @@ -88,6 +88,7 @@ obj-$(CONFIG_TOUCHSCREEN_SUR40) += sur40.o
> > obj-$(CONFIG_TOUCHSCREEN_SURFACE3_SPI) += surface3_spi.o
> > obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC) += ti_am335x_tsc.o
> > obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213) += touchit213.o
> > +obj-$(CONFIG_TOUCHSCREEN_TOUCHNETIX_AXIOM) += touchnetix_axiom.o
> > obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT) += touchright.o
> > obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN) += touchwin.o
> > obj-$(CONFIG_TOUCHSCREEN_TS4800) += ts4800-ts.o
> > diff --git a/drivers/input/touchscreen/touchnetix_axiom.c b/drivers/input/touchscreen/touchnetix_axiom.c
> > new file mode 100644
> > index 000000000000..9783e89f9e3a
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/touchnetix_axiom.c
> > @@ -0,0 +1,675 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * TouchNetix axiom Touchscreen Driver
> > + *
> > + * Copyright (C) 2020-2023 TouchNetix Ltd.
> > + *
> > + * Author(s): Bart Prescott <bartp@baasheep.co.uk>
> > + * Pedro Torruella <pedro.torruella@touchnetix.com>
> > + * Mark Satterthwaite <mark.satterthwaite@touchnetix.com>
> > + * Hannah Rossiter <hannah.rossiter@touchnetix.com>
> > + * Kamel Bouhara <kamel.bouhara@bootlin.com>
> > + *
> > + */
> > +#include <linux/bitfield.h>
> > +#include <linux/crc16.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/i2c.h>
> > +#include <linux/input.h>
> > +#include <linux/input/mt.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +
> > +#define AXIOM_PROX_LEVEL -128
> > +/*
> > + * Register group u31 has 2 pages for usage table entries.
> > + */
> > +#define AXIOM_U31_MAX_USAGES ((2 * AXIOM_COMMS_PAGE_SIZE) / AXIOM_U31_BYTES_PER_USAGE)
> > +#define AXIOM_U31_BYTES_PER_USAGE 6
> > +#define AXIOM_U31_PAGE0_LENGTH 0x0C
> > +#define AXIOM_U31_BOOTMODE_MASK BIT(7)
> > +#define AXIOM_U31_DEVID_MASK GENMASK(14, 0)
> > +
> > +#define AXIOM_CMD_HEADER_READ_MASK BIT(15)
> > +#define AXIOM_U41_MAX_TARGETS 10
> > +
> > +#define AXIOM_U46_AUX_CHANNELS 4
> > +#define AXIOM_U46_AUX_MASK GENMASK(11, 0)
> > +
> > +#define AXIOM_COMMS_MAX_USAGE_PAGES 3
> > +#define AXIOM_COMMS_PAGE_SIZE 256
> > +#define AXIOM_COMMS_REPORT_LEN_MASK GENMASK(6, 0)
> > +
> > +#define AXIOM_REPORT_USAGE_ID 0x34
> > +#define AXIOM_DEVINFO_USAGE_ID 0x31
> > +#define AXIOM_USAGE_2HB_REPORT_ID 0x01
> > +#define AXIOM_USAGE_2AUX_REPORT_ID 0x46
> > +#define AXIOM_USAGE_2DCTS_REPORT_ID 0x41
> > +
> > +#define AXIOM_PAGE_OFFSET_MASK GENMASK(6, 0)
> > +
> > +struct axiom_devinfo {
> > + u16 device_id;
> > + u8 fw_minor;
> > + u8 fw_major;
> > + u8 fw_info_extra;
> > + u8 tcp_revision;
> > + u8 bootloader_fw_minor;
> > + u8 bootloader_fw_major;
> > + u16 jedec_id;
> > + u8 num_usages;
> > +} __packed;
> > +
> > +/*
> > + * Describes parameters of a specific usage, essentially a single element of
> > + * the "Usage Table"
> > + */
> > +struct axiom_usage_entry {
> > + u8 id;
> > + u8 is_report;
> > + u8 start_page;
> > + u8 num_pages;
> > +};
> > +
> > +/*
> > + * Represents state of a touch or target when detected prior a touch (eg.
> > + * hover or proximity events).
> > + */
> > +enum axiom_target_state {
> > + AXIOM_TARGET_STATE_NOT_PRESENT = 0,
> > + AXIOM_TARGET_STATE_PROX = 1,
> > + AXIOM_TARGET_STATE_HOVER = 2,
> > + AXIOM_TARGET_STATE_TOUCHING = 3,
> > +};
> > +
> > +struct axiom_u41_target {
> > + enum axiom_target_state state;
> > + u16 x;
> > + u16 y;
> > + s8 z;
> > + bool insert;
> > + bool touch;
> > +};
> > +
> > +struct axiom_target_report {
> > + u8 index;
> > + u8 present;
> > + u16 x;
> > + u16 y;
> > + s8 z;
> > +};
> > +
> > +struct axiom_cmd_header {
> > + __le16 target_address;
> > + __le16 length;
> > +} __packed;
> > +
> > +struct axiom_data {
> > + struct axiom_devinfo devinfo;
> > + struct device *dev;
> > + struct gpio_desc *reset_gpio;
> > + struct i2c_client *client;
> > + struct input_dev *input_dev;
> > + u32 max_report_len;
> > + char rx_buf[AXIOM_COMMS_MAX_USAGE_PAGES * AXIOM_COMMS_PAGE_SIZE];
> > + struct axiom_u41_target targets[AXIOM_U41_MAX_TARGETS];
> > + struct axiom_usage_entry usage_table[AXIOM_U31_MAX_USAGES];
> > + bool usage_table_populated;
> > + struct regulator *vdda;
> > + struct regulator *vddi;
> > +};
> > +
> > +/*
> > + * axiom devices are typically configured to report
> > + * touches at a rate of 100Hz (10ms). For systems
> > + * that require polling for reports.
> > + * When reports are polled, it will be expected to
> > + * occasionally observe the overflow bit being set
> > + * in the reports. This indicates that reports are not
> > + * being read fast enough.
> > + */
> > +#define POLL_INTERVAL_DEFAULT_MS 10
> > +
> > +/* Translate usage/page/offset triplet into physical address. */
> > +static u16 axiom_usage_to_target_address(struct axiom_data *ts, char usage, char page,
> > + char offset)
> > +{
> > + struct axiom_usage_entry *usage_table;
> > + struct axiom_devinfo *device_info;
> > + u32 i;
> > +
> > + device_info = &ts->devinfo;
> > + usage_table = ts->usage_table;
> > +
> > + /* At the moment the convention is that u31 is always at physical address 0x0 */
> > + if (!ts->usage_table_populated) {
> > + if (usage == AXIOM_DEVINFO_USAGE_ID)
> > + return ((page << 8) + offset);
> > + else
> > + return 0xffff;
> > + }
> > +
> > + for (i = 0; i < ts->devinfo.num_usages; i++) {
> > + if (usage_table[i].id != usage)
> > + continue;
> > +
> > + if (page >= usage_table[i].num_pages) {
> > + dev_err(ts->dev, "Invalid usage table! usage: %u, page: %u, offset: %u\n",
> > + usage, page, offset);
> > + return 0xffff;
> > + }
> > + break;
> > + }
> > +
> > + return ((usage_table[i].start_page + page) << 8) + offset;
> > +}
> > +
> > +static int axiom_i2c_read(struct i2c_client *client, u8 usage, u8 page, u8 *buf, u16 len)
> > +{
> > + struct axiom_data *ts = i2c_get_clientdata(client);
> > + struct axiom_cmd_header cmd_header;
> > + u8 buffer[sizeof(cmd_header) + AXIOM_COMMS_PAGE_SIZE];
> > + struct i2c_msg msg;
> > + int error;
> > +
> > + cmd_header.target_address = cpu_to_le16(axiom_usage_to_target_address(ts, usage, page, 0));
> > + cmd_header.length = cpu_to_le16(len | AXIOM_CMD_HEADER_READ_MASK);
> > +
> > + memcpy(&buffer[0], &cmd_header, sizeof(cmd_header));
> > + memcpy(&buffer[sizeof(cmd_header)], buf, len);
>
> You're copying a buffer that is meant to hold the response which makes
> no sense.
> Please stick to the original implementation which transfers 2 messages :
> - first is the header (you're telling the controller what you want to
> read) : this is a write and flags should be 0.
> - second is the actual read message : flags should be I2C_M_RD
This was not supposed to be applied on read but rather axiom_i2c_write().
Fixed for v5, thanks.
>
> > +
> > + msg.addr = client->addr;
> > + msg.flags = 0;
> > + msg.len = len + sizeof(cmd_header);
> > + msg.buf = (u8 *)buffer;
> > +
> > + error = i2c_transfer(client->adapter, &msg, 1);
> > + if (error != 1) {
> You should transfer 2 messages (see above).
> > + dev_err(&client->dev,
> > + "Failed reading usage %#x page %#x, error=%d\n",
> > + usage, page, error);
> > + return error;
> > + }
> > +
> > + usleep_range(100, 250);
> The delay here is not enough as Touchnetix confirmed that we would need
> 250 usecs for the controller's internal DMA to be ready after an i2c
> transfer.
Ok that wasn't clear if it was after 2 consecutives reads but now it is.
Maybe Mark can confirm again ?
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * One of the main purposes for reading the usage table is to identify
> > + * which usages reside at which TA address.
> > + * When performing subsequent reads or writes to AXIOM, the TA is used
> > + * to specify which usage is being accessed.
> > + * Consider the following discovery code which will build up the usage table.
> > + */
> > +static u32 axiom_populate_usage_table(struct axiom_data *ts)
> > +{
> > + struct axiom_usage_entry *usage_table;
> > + u32 max_report_len = 0;
> > + char *rx_data = ts->rx_buf;
> > + u32 usage_id;
> > + int error;
> > +
> > + usage_table = ts->usage_table;
> > +
> > + /* Read the second page of usage u31 to get the usage table */
> > + error = axiom_i2c_read(ts->client, AXIOM_DEVINFO_USAGE_ID, 1, rx_data,
> > + (AXIOM_U31_BYTES_PER_USAGE * ts->devinfo.num_usages));
> > + if (error)
> > + return error;
> > +
> > + for (usage_id = 0; usage_id < ts->devinfo.num_usages; usage_id++) {
> > + u16 offset = (usage_id * AXIOM_U31_BYTES_PER_USAGE);
> > + u8 id = rx_data[offset + 0];
> > + u8 start_page = rx_data[offset + 1];
> > + u8 num_pages = rx_data[offset + 2];
> > + u32 max_offset = ((rx_data[offset + 3] & AXIOM_PAGE_OFFSET_MASK) + 1) * 2;
> > +
> > + if (!num_pages)
> > + usage_table[usage_id].is_report = true;
> > +
> > + /* Store the entry into the usage table */
> > + usage_table[usage_id].id = id;
> > + usage_table[usage_id].start_page = start_page;
> > + usage_table[usage_id].num_pages = num_pages;
> > +
> > + dev_dbg(ts->dev, "Usage u%02x Info: %*ph\n", id,
> > + AXIOM_U31_BYTES_PER_USAGE, &rx_data[offset]);
> > +
> > + /* Identify the max report length the module will receive */
> > + if (usage_table[usage_id].is_report && max_offset > max_report_len)
> > + max_report_len = max_offset;
> > + }
> > +
> > + ts->usage_table_populated = true;
> > +
> > + return max_report_len;
> > +}
> > +
> > +static int axiom_discover(struct axiom_data *ts)
> > +{
> > + int error;
> > +
> > + /*
> > + * Fetch the first page of usage u31 to get the
> > + * device information and the number of usages
> > + */
> > + error = axiom_i2c_read(ts->client, AXIOM_DEVINFO_USAGE_ID, 0, (char *)&ts->devinfo,
> > + AXIOM_U31_PAGE0_LENGTH);
> > + if (error)
> > + return error;
> > +
> > + dev_dbg(ts->dev, " Boot Mode : %s\n",
> > + FIELD_GET(AXIOM_U31_BOOTMODE_MASK, ts->devinfo.device_id) ? "BLP" : "TCP");
> > + dev_dbg(ts->dev, " Device ID : %04lx\n",
> > + FIELD_GET(AXIOM_U31_DEVID_MASK, ts->devinfo.device_id));
> > + dev_dbg(ts->dev, " Firmware Rev : %02x.%02x\n", ts->devinfo.fw_major,
> > + ts->devinfo.fw_minor);
> > + dev_dbg(ts->dev, " Bootloader Rev : %02x.%02x\n", ts->devinfo.bootloader_fw_major,
> > + ts->devinfo.bootloader_fw_minor);
> > + dev_dbg(ts->dev, " FW Extra Info : %04x\n", ts->devinfo.fw_info_extra);
> > + dev_dbg(ts->dev, " Silicon : %04x\n", ts->devinfo.jedec_id);
> > + dev_dbg(ts->dev, " Number usages : %04x\n", ts->devinfo.num_usages);
> > +
> > + ts->max_report_len = axiom_populate_usage_table(ts);
> > + if (!ts->max_report_len || !ts->devinfo.num_usages)
> > + return -EINVAL;
> > +
> > + dev_dbg(ts->dev, "Max Report Length: %u\n", ts->max_report_len);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Support function to axiom_process_u41_report.
> > + * Generates input-subsystem events for every target.
> > + * After calling this function the caller shall issue
> > + * a Sync to the input sub-system.
> > + */
> > +static bool axiom_process_u41_report_target(struct axiom_data *ts,
> > + struct axiom_target_report *target)
> > +{
> > + struct input_dev *input_dev = ts->input_dev;
> > + struct axiom_u41_target *target_prev_state;
> > + enum axiom_target_state current_state;
> > + bool update = false;
> > + int slot;
> > +
> > + /* Verify the target index */
> > + if (target->index >= AXIOM_U41_MAX_TARGETS) {
> > + dev_dbg(ts->dev, "Invalid target index! %u\n", target->index);
> > + return false;
> > + }
> > +
> > + target_prev_state = &ts->targets[target->index];
> > +
> > + current_state = AXIOM_TARGET_STATE_NOT_PRESENT;
> > +
> > + if (target->present) {
> > + if (target->z >= 0)
> > + current_state = AXIOM_TARGET_STATE_TOUCHING;
> > + else if (target->z > AXIOM_PROX_LEVEL && target->z < 0)
> > + current_state = AXIOM_TARGET_STATE_HOVER;
> > + else if (target->z == AXIOM_PROX_LEVEL)
> > + current_state = AXIOM_TARGET_STATE_PROX;
> > + }
> > +
> > + if (target_prev_state->state == current_state &&
> > + target_prev_state->x == target->x &&
> > + target_prev_state->y == target->y &&
> > + target_prev_state->z == target->z) {
> > + return false;
> > + }
> > +
> > + slot = target->index;
> > +
> > + dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
> > + target->index, slot, target->present,
> > + target->x, target->y, target->z);
> > +
> > + switch (current_state) {
> > + case AXIOM_TARGET_STATE_NOT_PRESENT:
> > + case AXIOM_TARGET_STATE_PROX:
> > + if (!target_prev_state->insert)
> > + break;
> > + update = true;
> > + target_prev_state->insert = false;
> > + input_mt_slot(input_dev, slot);
> > +
> > + if (!slot)
> > + input_report_key(input_dev, BTN_TOUCH, 0);
> > +
> > + input_mt_report_slot_inactive(input_dev);
> > + /*
> > + * make sure the previous coordinates are
> > + * all off screen when the finger comes back
> > + */
> > + target->x = 65535;
> > + target->y = 65535;
> > + target->z = AXIOM_PROX_LEVEL;
> > + break;
> > + case AXIOM_TARGET_STATE_HOVER:
> > + case AXIOM_TARGET_STATE_TOUCHING:
> > + target_prev_state->insert = true;
> > + update = true;
> > + input_mt_slot(input_dev, slot);
> > + input_report_abs(input_dev, ABS_MT_TRACKING_ID, slot);
> > + input_report_abs(input_dev, ABS_MT_POSITION_X, target->x);
> > + input_report_abs(input_dev, ABS_X, target->x);
> > + input_report_abs(input_dev, ABS_MT_POSITION_Y, target->y);
> > + input_report_abs(input_dev, ABS_Y, target->y);
> > +
> > + if (current_state == AXIOM_TARGET_STATE_TOUCHING) {
> > + input_report_abs(input_dev, ABS_MT_DISTANCE, 0);
> > + input_report_abs(input_dev, ABS_DISTANCE, 0);
> > + input_report_abs(input_dev, ABS_MT_PRESSURE, target->z);
> > + input_report_abs(input_dev, ABS_PRESSURE, target->z);
> > + } else {
> > + input_report_abs(input_dev, ABS_MT_DISTANCE, -target->z);
> > + input_report_abs(input_dev, ABS_DISTANCE, -target->z);
> > + input_report_abs(input_dev, ABS_MT_PRESSURE, 0);
> > + input_report_abs(input_dev, ABS_PRESSURE, 0);
> > + }
> > +
> > + if (!slot)
> > + input_report_key(input_dev, BTN_TOUCH, (current_state ==
> > + AXIOM_TARGET_STATE_TOUCHING));
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + target_prev_state->state = current_state;
> > + target_prev_state->x = target->x;
> > + target_prev_state->y = target->y;
> > + target_prev_state->z = target->z;
> > +
> > + return update;
> > +}
> > +
> > +/*
> > + * U41 is the output report of the 2D CTS and contains the status of targets
> > + * (including contacts and pre-contacts) along with their X,Y,Z values.
> > + * When a target has been removed (no longer detected),
> > + * the corresponding X,Y,Z values will be zeroed.
> > + */
> > +static bool axiom_process_u41_report(struct axiom_data *ts, char *rx_buf)
> > +{
> > + struct axiom_target_report target;
> > + bool update_done = false;
> > + u16 target_status;
> > + u32 i;
> > +
> > + target_status = ((rx_buf[1]) | (rx_buf[2] << 8));
> > +
> > + for (i = 0; i < AXIOM_U41_MAX_TARGETS; i++) {
> > + char target_step = i * 4;
> > +
> > + target.index = i;
> > + target.present = ((target_status & (1 << i)) != 0) ? 1 : 0;
> > + target.x = (rx_buf[(target_step + 3)] | (rx_buf[target_step + 4] << 8));
> > + target.y = (rx_buf[(target_step + 5)] | (rx_buf[target_step + 6] << 8));
> > + target.z = (s8)(rx_buf[i + 43]);
> > + update_done |= axiom_process_u41_report_target(ts, &target);
> > + }
> > +
> > + return update_done;
> > +}
> > +
> > +/*
> > + * U46 report contains a low level measurement data generated by the CDS
> > + * algorithms from the AUX channels. This information is useful when tuning
> > + * multi-press to assess mechanical consistency in the unit's construction.
> > + */
> > +static void axiom_process_u46_report(struct axiom_data *ts, char *rx_buf)
> > +{
> > + struct input_dev *input_dev = ts->input_dev;
> > + u32 event_value;
> > + u16 aux_value;
> > + u32 i = 0;
> > +
> > + for (i = 0; i < AXIOM_U46_AUX_CHANNELS; i++) {
> > + char target_step = i * 2;
> > +
> > + aux_value = ((rx_buf[target_step + 2] << 8) | (rx_buf[target_step + 1]))
> > + & AXIOM_U46_AUX_MASK;
> > + event_value = (i << 16) | (aux_value);
> > + input_event(input_dev, EV_MSC, MSC_RAW, event_value);
> > + }
> > +}
> > +
> > +/*
> > + * Validates the crc and demultiplexes the axiom reports to the appropriate
> > + * report handler
> > + */
> > +static int axiom_handle_events(struct axiom_data *ts)
> > +{
> > + struct input_dev *input_dev = ts->input_dev;
> > + char *report_data = ts->rx_buf;
> > + char report_len;
> > + struct device *dev = ts->dev;
> > + u16 crc_report;
> > + u16 crc_calc;
> > + int error;
> > +
> > + error = axiom_i2c_read(ts->client, AXIOM_REPORT_USAGE_ID, 0, &report_len,
> > + sizeof(report_len));
>
> I know that I told you it would be better to do the read report in 2
> phases, but we should have stick to Touchnetix recommendation that is to
> read the whole report even if it's padded with zeroes.
> This is to avoid the 250 usecs delay after the read of the report length.
>
Ack, thanks.
> > + if (error)
> > + return error;
> > +
> > + report_len = (report_len & AXIOM_COMMS_REPORT_LEN_MASK) << 1;
> > + if (!report_len) {
> > + dev_err(dev, "Zero length report discarded\n");
> > + return -ENODATA;
> > + }
> > +
> > + error = axiom_i2c_read(ts->client, AXIOM_REPORT_USAGE_ID, 0, report_data, report_len);
> > + if (error)
> > + return error;
> > +
> > + /* Validate the report CRC */
> > + crc_report = (report_data[report_len - 1] << 8) | (report_data[report_len - 2]);
> > + /* Length is in 16 bit words and remove the size of the CRC16 itself */
> > + crc_calc = crc16(0, report_data, (report_len - 2));
> > +
> > + if (crc_calc != crc_report) {
> > + dev_err(dev,
> > + "CRC mismatch! Expected: %#x, Calculated CRC: %#x.\n",
> > + crc_report, crc_calc);
> > + return -EINVAL;
> > + }
> > +
> > + switch (report_data[1]) {
> > + case AXIOM_USAGE_2DCTS_REPORT_ID:
> > + if (axiom_process_u41_report(ts, &report_data[1])) {
> > + input_mt_sync_frame(input_dev);
> > + input_sync(input_dev);
> > + }
> > + break;
> > +
> > + case AXIOM_USAGE_2AUX_REPORT_ID:
> > + /* This is an aux report (force) */
> > + axiom_process_u46_report(ts, &report_data[1]);
> > + input_mt_sync(input_dev);
> > + input_sync(input_dev);
> > + break;
> > +
> > + case AXIOM_USAGE_2HB_REPORT_ID:
> > + /* This is a heartbeat report */
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void axiom_i2c_poll(struct input_dev *input_dev)
> > +{
> > + struct axiom_data *ts = input_get_drvdata(input_dev);
> > +
> > + axiom_handle_events(ts);
> > +}
> > +
> > +static irqreturn_t axiom_irq(int irq, void *dev_id)
> > +{
> > + struct axiom_data *ts = dev_id;
> > +
> > + axiom_handle_events(ts);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void axiom_reset(struct gpio_desc *reset_gpio)
> > +{
> > + gpiod_set_value_cansleep(reset_gpio, 1);
> > + usleep_range(1000, 2000);
> > + gpiod_set_value_cansleep(reset_gpio, 0);
> > + msleep(110);
> > +}
> > +
> > +static int axiom_i2c_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct input_dev *input_dev;
> > + struct axiom_data *ts;
> > + u32 startup_delay_ms;
> > + u32 poll_interval;
> > + int target;
> > + int error;
> > +
> > + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> > + if (!ts)
> > + return -ENOMEM;
> > +
> > + ts->client = client;
> > + i2c_set_clientdata(client, ts);
> > + ts->dev = dev;
> > +
> > + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > + if (IS_ERR(ts->reset_gpio))
> > + return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
> > +
> > + if (ts->reset_gpio)
> > + axiom_reset(ts->reset_gpio);
> > +
> > + ts->vddi = devm_regulator_get_optional(dev, "VDDI");
> > + if (!IS_ERR(ts->vddi)) {
> > + error = regulator_enable(ts->vddi);
> > + if (error)
> > + return dev_err_probe(&client->dev, PTR_ERR(ts->vddi),
> > + "Failed to enable VDDI regulator\n");
> > + }
> > +
> > + ts->vdda = devm_regulator_get_optional(dev, "VDDA");
> > + if (!IS_ERR(ts->vdda)) {
> > + error = regulator_enable(ts->vdda);
> > + if (error) {
> > + dev_err(dev, "Failed to get VDDA regulator\n");
> > + regulator_disable(ts->vddi);
> > + return error;
> > + }
> > + if (!device_property_read_u32(dev, "startup-time-ms", &startup_delay_ms))
> > + msleep(startup_delay_ms);
> > + }
> > +
> > + error = axiom_discover(ts);
> > + if (error)
> > + return dev_err_probe(dev, error, "Failed touchscreen discover\n");
> > +
> > + input_dev = devm_input_allocate_device(ts->dev);
> > + if (!input_dev)
> > + return -ENOMEM;
> > +
> > + input_dev->name = "TouchNetix axiom Touchscreen";
> > + input_dev->phys = "input/axiom_ts";
> > +
> > + /* Single Touch */
> > + input_set_abs_params(input_dev, ABS_X, 0, 65535, 0, 0);
> > + input_set_abs_params(input_dev, ABS_Y, 0, 65535, 0, 0);
> > +
> > + /* Multi Touch */
> > + /* Min, Max, Fuzz (expected noise in px, try 4?) and Flat */
> > + input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, 65535, 0, 0);
> > + /* Min, Max, Fuzz (expected noise in px, try 4?) and Flat */
> > + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, 65535, 0, 0);
> > + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
> > + input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
> > + input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
> > +
> > + input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
> > +
> > + /* Registers the axiom device as a touchscreen instead of a mouse pointer */
> > + error = input_mt_init_slots(input_dev, AXIOM_U41_MAX_TARGETS, INPUT_MT_DIRECT);
> > + if (error)
> > + return error;
> > +
> > + /* Enables the raw data for up to 4 force channels to be sent to the input subsystem */
> > + set_bit(EV_REL, input_dev->evbit);
> > + set_bit(EV_MSC, input_dev->evbit);
> > + /* Declare that we support "RAW" Miscellaneous events */
> > + set_bit(MSC_RAW, input_dev->mscbit);
> > +
> > + ts->input_dev = input_dev;
> > + input_set_drvdata(ts->input_dev, ts);
> > +
> > + /* Ensure that all reports are initialised to not be present. */
> > + for (target = 0; target < AXIOM_U41_MAX_TARGETS; target++)
> > + ts->targets[target].state = AXIOM_TARGET_STATE_NOT_PRESENT;
> > +
> > + error = input_register_device(input_dev);
> > + if (error)
> > + return dev_err_probe(ts->dev, error,
> > + "Could not register with Input Sub-system.\n");
> > +
> > + error = devm_request_threaded_irq(dev, client->irq, NULL,
> > + axiom_irq, IRQF_ONESHOT, dev_name(dev), ts);
> > + if (error < 0) {
> > + dev_warn(dev, "Request irq failed, falling back to polling mode");
> > +
> > + error = input_setup_polling(input_dev, axiom_i2c_poll);
> > + if (error)
> > + return dev_err_probe(ts->dev, error, "Unable to set up polling mode\n");
> > +
> > + if (!device_property_read_u32(ts->dev, "poll-interval", &poll_interval))
> > + input_set_poll_interval(input_dev, poll_interval);
> > + else
> > + input_set_poll_interval(input_dev, POLL_INTERVAL_DEFAULT_MS);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct i2c_device_id axiom_i2c_id_table[] = {
> > + { "ax54a" },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
> > +
> > +static const struct of_device_id axiom_i2c_of_match[] = {
> > + { .compatible = "touchnetix,ax54a", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, axiom_i2c_of_match);
> > +
> > +static struct i2c_driver axiom_i2c_driver = {
> > + .driver = {
> > + .name = "axiom",
> > + .of_match_table = axiom_i2c_of_match,
> > + },
> > + .id_table = axiom_i2c_id_table,
> > + .probe = axiom_i2c_probe,
> > +};
> > +module_i2c_driver(axiom_i2c_driver);
> > +MODULE_AUTHOR("Bart Prescott <bartp@baasheep.co.uk>");
> > +MODULE_AUTHOR("Pedro Torruella <pedro.torruella@touchnetix.com>");
> > +MODULE_AUTHOR("Mark Satterthwaite <mark.satterthwaite@touchnetix.com>");
> > +MODULE_AUTHOR("Hannah Rossiter <hannah.rossiter@touchnetix.com>");
> > +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
> > +MODULE_DESCRIPTION("TouchNetix axiom touchscreen I2C bus driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1
> >
>
--
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v3 4/7] HID: i2c-hid: Move i2c_hid_finish_hwreset() to after reading the report-descriptor
From: Doug Anderson @ 2023-12-04 16:28 UTC (permalink / raw)
To: Hans de Goede
Cc: Jiri Kosina, Benjamin Tissoires, Julian Sax, ahormann,
Bruno Jesus, Dietrich, kloxdami, Tim Aldridge, Rene Wagner,
Federico Ricchiuto, linux-input
In-Reply-To: <20231202224615.24818-5-hdegoede@redhat.com>
Hi,
On Sat, Dec 2, 2023 at 2:46 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> A recent bug made me look at Microsoft's i2c-hid docs again
> and I noticed the following:
>
> """
> 4. Issue a RESET (Host Initiated Reset) to the Device.
> 5. Retrieve report descriptor from the device.
>
> Note: Steps 4 and 5 may be done in parallel to optimize for time on I²C.
> Since report descriptors are (a) static and (b) quite long, Windows 8 may
> issue a request for 5 while it is waiting for a response from the device
> on 4.
> """
>
> Which made me think that maybe on some touchpads the reset ack is delayed
> till after the report descriptor is read ?
>
> Testing a T-BAO Tbook Air 12.5 with a 0911:5288 (SIPODEV SP1064?) touchpad,
> for which the I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirk was first introduced,
> shows that reading the report descriptor before waiting for the reset
> helps with the missing reset IRQ. Now the reset does get acked properly,
> but the ack sometimes still does not happen unfortunately.
>
> Still moving the wait for ack to after reading the report-descriptor,
> is probably a good idea, both to make i2c-hid's behavior closer to
> Windows as well as to speed up probing i2c-hid devices.
>
> While at it drop the dbg_hid() for a malloc failure, malloc failures
> already get logged extensively by malloc itself.
>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2247751
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v3:
> - Use goto abort_reset instead of return on i2c_hid_start_hwreset()
> failure, so that the mutex gets properly unlocked
>
> Changes in v2:
> - Adjust commit message to note that moving the wait-for-reset
> to after reading thr report-descriptor only partially fixes
> the missing reset IRQ problem
> - Adjust for the reset_lock now being taken in the callers of
> i2c_hid_start_hwreset() / i2c_hid_finish_hwreset()
> ---
> drivers/hid/i2c-hid/i2c-hid-core.c | 29 +++++++++++++++++++----------
> 1 file changed, 19 insertions(+), 10 deletions(-)
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail
From: Doug Anderson @ 2023-12-04 16:50 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Wolfram Sang,
Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAGXv+5E+R292XsOFSL-j0KJMmVJjWtxMRgCK8besP7mo6NDOWA@mail.gmail.com>
Hi,
On Sun, Dec 3, 2023 at 10:59 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > >
> > > @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
> > > reg = <0x2c>;
> > > hid-descr-addr = <0x0020>;
> > > wakeup-source;
> > > + status = "fail-needs-probe";
> >
> > While doing this, you could also remove the hack where the trackpad
> > IRQ pinctrl is listed under i2c4.
>
> Sure. I do think we can do away with it though. According to at least one
> schematic, the interrupt line has pull-ups on both sides of the voltage
> shifter.
>
> BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
> both sides of the voltage shifter as well.
I dunno if the convention is different on Mediatek boards, but at
least on boards I've been involved with in the past we've always put
pinctrl entries just to make things explicit. This meant that we
didn't rely on the firmware/bootrom/defaults to leave pulls in any
particular state. ...otherwise those external pull-ups could be
fighting with internal pull-downs, right?
-Doug
^ permalink raw reply
* [PATCH v3.1 0/8] Convert DA906{1,2} bindings to json-schema
From: Biju Das @ 2023-12-04 17:25 UTC (permalink / raw)
To: Lee Jones, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Biju Das, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Steve Twiss, linux-input, devicetree,
linux-pm, Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
linux-renesas-soc
Convert the below bindings to json-schema
1) DA906{1,2} mfd bindings
2) DA906{1,2,3} onkey bindings
3) DA906{1,2,3} thermal bindings
Also add fallback for DA9061 watchdog device and document
DA9063 watchdog device.
v3->v3.1:
* Patch#1 is merge of patch#1 from v2 + patch#8 from v2.
* Dropped comment for d9061 watchdog fallback
* Replaced enum->const for dlg,da9061-watchdog and its fallback.
* Restored patch#4 in series 1 and dropped the thermal example
* Added Ack from Conor Dooley for da9063 watchdog binding support.
* Updated title DA9062/61->DA906{1,2,3} as it supports DA9063.
* Retained Rb tag since the changes are trivial.
* Added Ack from Conor for updating watchdog property
* Dropped link to product information.
* Patch#5(onkey) is squashed with patch#6 and patch#9 from v2.
* Replaced enum->const for dlg,da9061-onkey and its fallback.
* Dropped example
* Restored the thermal binding patch from v2.
* Dropped example
* Replaced enum->const for compatible property.
* Added Rb tag from Rob and retained Rb tag as changes are trivial.
* Added Ack from Conor Dooley for patch#7.
* Split the thermal binding patch separate
* Updated the description
v2->v3:
* Updated Maintainer entries for watchdog,onkey and thermal bindings
* Fixed bot errors related to MAINTAINERS entry, invalid doc
references and thermal examples by merging patch#4.
v1->v2:
Link: https://lore.kernel.org/all/20231201110840.37408-5-biju.das.jz@bp.renesas.com/
* DA9062 and DA9061 merged with DA9063
* Sorted the child devices
* mfd,onkey and thermal are pointing to child bindings
Biju Das (8):
dt-bindings: mfd: da9062: Update watchdog description
dt-bindings: watchdog: dlg,da9062-watchdog: Add fallback for DA9061
watchdog
dt-bindings: watchdog: dlg,da9062-watchdog: Document DA9063 watchdog
dt-bindings: mfd: dlg,da9063: Update watchdog property
dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
dt-bindings: thermal: Convert da906{1,2} thermal to json-schema
dt-bindings: mfd: dlg,da9063: Sort child devices
dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
.../bindings/input/da9062-onkey.txt | 47 ----
.../bindings/input/dlg,da9062-onkey.yaml | 39 ++++
.../devicetree/bindings/mfd/da9062.txt | 124 ----------
.../devicetree/bindings/mfd/dlg,da9063.yaml | 221 +++++++++++++++---
.../bindings/thermal/da9062-thermal.txt | 36 ---
.../bindings/thermal/dlg,da9062-thermal.yaml | 35 +++
.../watchdog/dlg,da9062-watchdog.yaml | 13 +-
MAINTAINERS | 6 +-
8 files changed, 272 insertions(+), 249 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/da9062-onkey.txt
create mode 100644 Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
delete mode 100644 Documentation/devicetree/bindings/thermal/da9062-thermal.txt
create mode 100644 Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
--
2.39.2
^ permalink raw reply
* [PATCH v3.1 5/8] dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
From: Biju Das @ 2023-12-04 17:25 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Biju Das, Support Opensource, linux-input, devicetree,
Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
linux-renesas-soc
In-Reply-To: <20231204172510.35041-1-biju.das.jz@bp.renesas.com>
Convert the da906{1,2,3} onkey device tree binding documentation to
json-schema.
Update MAINTAINERS entries, description and onkey property by
referring to dlg,da9062-onkey binding file.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v3.1:
* Squashed with patch#6 and patch#9 from v2.
* Replaced enum->const for dlg,da9061-onkey and its fallback.
* Dropped example
v2->v3:
* Updated MAINTAINERS entries.
v2:
* New patch
---
.../bindings/input/da9062-onkey.txt | 47 -------------------
.../bindings/input/dlg,da9062-onkey.yaml | 40 ++++++++++++++++
.../devicetree/bindings/mfd/da9062.txt | 2 +-
.../devicetree/bindings/mfd/dlg,da9063.yaml | 15 +-----
MAINTAINERS | 2 +-
5 files changed, 43 insertions(+), 63 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/da9062-onkey.txt
create mode 100644 Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
diff --git a/Documentation/devicetree/bindings/input/da9062-onkey.txt b/Documentation/devicetree/bindings/input/da9062-onkey.txt
deleted file mode 100644
index e5eef59a93dc..000000000000
--- a/Documentation/devicetree/bindings/input/da9062-onkey.txt
+++ /dev/null
@@ -1,47 +0,0 @@
-* Dialog DA9061/62/63 OnKey Module
-
-This module is part of the DA9061/DA9062/DA9063. For more details about entire
-DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
-For DA9063 see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
-
-This module provides the KEY_POWER event.
-
-Required properties:
-
-- compatible: should be one of the following valid compatible string lines:
- "dlg,da9061-onkey", "dlg,da9062-onkey"
- "dlg,da9062-onkey"
- "dlg,da9063-onkey"
-
-Optional properties:
-
-- dlg,disable-key-power : Disable power-down using a long key-press. If this
- entry exists the OnKey driver will remove support for the KEY_POWER key
- press when triggered using a long press of the OnKey.
-
-Example: DA9063
-
- pmic0: da9063@58 {
- onkey {
- compatible = "dlg,da9063-onkey";
- dlg,disable-key-power;
- };
- };
-
-Example: DA9062
-
- pmic0: da9062@58 {
- onkey {
- compatible = "dlg,da9062-onkey";
- dlg,disable-key-power;
- };
- };
-
-Example: DA9061 using a fall-back compatible for the DA9062 onkey driver
-
- pmic0: da9061@58 {
- onkey {
- compatible = "dlg,da9061-onkey", "dlg,da9062-onkey";
- dlg,disable-key-power;
- };
- };
diff --git a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
new file mode 100644
index 000000000000..4cff91f4bd34
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/dlg,da9062-onkey.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dialog DA9061/62/63 OnKey Module
+
+maintainers:
+ - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+ This module is part of the DA9061/DA9062/DA9063. For more details about entire
+ DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
+ For DA9063 see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+
+ This module provides the KEY_POWER event.
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - dlg,da9062-onkey
+ - dlg,da9063-onkey
+ - items:
+ - const: dlg,da9061-onkey
+ - const: dlg,da9062-onkey
+
+ dlg,disable-key-power:
+ type: boolean
+ description:
+ Disable power-down using a long key-press. If this entry exists
+ the OnKey driver will remove support for the KEY_POWER key press
+ when triggered using a long press of the OnKey.
+
+required:
+ - compatible
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
index 18463b7fbb42..154c31fa4443 100644
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ b/Documentation/devicetree/bindings/mfd/da9062.txt
@@ -84,7 +84,7 @@ Sub-nodes:
with the DA9062. There are currently no entries in this binding, however
compatible = "dlg,da9062-rtc" should be added if a node is created.
-- onkey : See ../input/da9062-onkey.txt
+- onkey : See ../input/dlg,da9062-onkey.yaml
- watchdog: See ../watchdog/dlg,da9062-watchdog.yaml
diff --git a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
index ce81e0b029cc..1e5a847a6be2 100644
--- a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+++ b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
@@ -44,20 +44,7 @@ properties:
const: dlg,da9063-rtc
onkey:
- type: object
- $ref: /schemas/input/input.yaml#
- unevaluatedProperties: false
- properties:
- compatible:
- const: dlg,da9063-onkey
-
- dlg,disable-key-power:
- type: boolean
- description: |
- Disable power-down using a long key-press.
- If this entry does not exist then by default the key-press triggered
- power down is enabled and the OnKey will support both KEY_POWER and
- KEY_SLEEP.
+ $ref: /schemas/input/dlg,da9062-onkey.yaml
regulators:
type: object
diff --git a/MAINTAINERS b/MAINTAINERS
index fa3965f1bf0e..ea171a18217c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6110,8 +6110,8 @@ DIALOG SEMICONDUCTOR DRIVERS
M: Support Opensource <support.opensource@diasemi.com>
S: Supported
W: http://www.dialog-semiconductor.com/products
-F: Documentation/devicetree/bindings/input/da90??-onkey.txt
F: Documentation/devicetree/bindings/input/dlg,da72??.txt
+F: Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
F: Documentation/devicetree/bindings/mfd/da90*.txt
F: Documentation/devicetree/bindings/mfd/dlg,da90*.yaml
F: Documentation/devicetree/bindings/regulator/da92*.txt
--
2.39.2
^ permalink raw reply related
* [PATCH v3.1 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
From: Biju Das @ 2023-12-04 17:25 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones
Cc: Biju Das, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Steve Twiss, linux-input, devicetree,
linux-pm, Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
linux-renesas-soc
In-Reply-To: <20231204172510.35041-1-biju.das.jz@bp.renesas.com>
Convert the da9062 PMIC device tree binding documentation to json-schema.
Update the example to match reality.
While at it, update description with link to product information.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v3.1:
* Split the thermal binding patch separate.
* Updated the description.
v2->v3:
* Fixed bot errors related to MAINTAINERS entry, invalid doc
references and thermal examples by merging patch#4.
v2:
* New patch
---
.../bindings/input/dlg,da9062-onkey.yaml | 3 +-
.../devicetree/bindings/mfd/da9062.txt | 124 ------------
.../devicetree/bindings/mfd/dlg,da9063.yaml | 186 +++++++++++++++++-
.../bindings/thermal/dlg,da9062-thermal.yaml | 2 +-
4 files changed, 183 insertions(+), 132 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
diff --git a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
index 4cff91f4bd34..18b6a3f02c07 100644
--- a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
+++ b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
@@ -11,8 +11,7 @@ maintainers:
description: |
This module is part of the DA9061/DA9062/DA9063. For more details about entire
- DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
- For DA9063 see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+ DA906{1,2,3} chips see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
This module provides the KEY_POWER event.
diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
deleted file mode 100644
index c8a7f119ac84..000000000000
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ /dev/null
@@ -1,124 +0,0 @@
-* Dialog DA9062 Power Management Integrated Circuit (PMIC)
-
-Product information for the DA9062 and DA9061 devices can be found here:
-- https://www.dialog-semiconductor.com/products/da9062
-- https://www.dialog-semiconductor.com/products/da9061
-
-The DA9062 PMIC consists of:
-
-Device Supply Names Description
------- ------------ -----------
-da9062-regulator : : LDOs & BUCKs
-da9062-rtc : : Real-Time Clock
-da9062-onkey : : On Key
-da9062-watchdog : : Watchdog Timer
-da9062-thermal : : Thermal
-da9062-gpio : : GPIOs
-
-The DA9061 PMIC consists of:
-
-Device Supply Names Description
------- ------------ -----------
-da9062-regulator : : LDOs & BUCKs
-da9062-onkey : : On Key
-da9062-watchdog : : Watchdog Timer
-da9062-thermal : : Thermal
-
-======
-
-Required properties:
-
-- compatible : Should be
- "dlg,da9062" for DA9062
- "dlg,da9061" for DA9061
-- reg : Specifies the I2C slave address (this defaults to 0x58 but it can be
- modified to match the chip's OTP settings).
-
-Optional properties:
-
-- gpio-controller : Marks the device as a gpio controller.
-- #gpio-cells : Should be two. The first cell is the pin number and the
- second cell is used to specify the gpio polarity.
-
-See Documentation/devicetree/bindings/gpio/gpio.txt for further information on
-GPIO bindings.
-
-- interrupts : IRQ line information.
-- interrupt-controller
-
-See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt for
-further information on IRQ bindings.
-
-Sub-nodes:
-
-- regulators : This node defines the settings for the LDOs and BUCKs.
- The DA9062 regulators are bound using their names listed below:
-
- buck1 : BUCK_1
- buck2 : BUCK_2
- buck3 : BUCK_3
- buck4 : BUCK_4
- ldo1 : LDO_1
- ldo2 : LDO_2
- ldo3 : LDO_3
- ldo4 : LDO_4
-
- The DA9061 regulators are bound using their names listed below:
-
- buck1 : BUCK_1
- buck2 : BUCK_2
- buck3 : BUCK_3
- ldo1 : LDO_1
- ldo2 : LDO_2
- ldo3 : LDO_3
- ldo4 : LDO_4
-
- The component follows the standard regulator framework and the bindings
- details of individual regulator device can be found in:
- Documentation/devicetree/bindings/regulator/regulator.txt
-
- regulator-initial-mode may be specified for buck regulators using mode values
- from include/dt-bindings/regulator/dlg,da9063-regulator.h.
-
-- rtc : This node defines settings required for the Real-Time Clock associated
- with the DA9062. There are currently no entries in this binding, however
- compatible = "dlg,da9062-rtc" should be added if a node is created.
-
-- onkey : See ../input/dlg,da9062-onkey.yaml
-
-- watchdog: See ../watchdog/dlg,da9062-watchdog.yaml
-
-- thermal : See ../thermal/dlg,da9062-thermal.yaml
-
-Example:
-
- pmic0: da9062@58 {
- compatible = "dlg,da9062";
- reg = <0x58>;
- interrupt-parent = <&gpio6>;
- interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
- interrupt-controller;
-
- rtc {
- compatible = "dlg,da9062-rtc";
- };
-
- regulators {
- DA9062_BUCK1: buck1 {
- regulator-name = "BUCK1";
- regulator-min-microvolt = <300000>;
- regulator-max-microvolt = <1570000>;
- regulator-min-microamp = <500000>;
- regulator-max-microamp = <2000000>;
- regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
- regulator-boot-on;
- };
- DA9062_LDO1: ldo1 {
- regulator-name = "LDO_1";
- regulator-min-microvolt = <900000>;
- regulator-max-microvolt = <3600000>;
- regulator-boot-on;
- };
- };
- };
-
diff --git a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
index 676b4f2566ae..54bb23dbc73f 100644
--- a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+++ b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
@@ -4,7 +4,7 @@
$id: http://devicetree.org/schemas/mfd/dlg,da9063.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: Dialog DA9063/DA9063L Power Management Integrated Circuit (PMIC)
+title: Dialog DA906{3L,3,2,1} Power Management Integrated Circuit (PMIC)
maintainers:
- Steve Twiss <stwiss.opensource@diasemi.com>
@@ -17,10 +17,17 @@ description: |
moment where all voltage monitors are disabled. Next, as da9063 only supports
UV *and* OV monitoring, both must be set to the same severity and value
(0: disable, 1: enable).
+ Product information for the DA906{3L,3,2,1} devices can be found here:
+ - https://www.dialog-semiconductor.com/products/da9063l
+ - https://www.dialog-semiconductor.com/products/da9063
+ - https://www.dialog-semiconductor.com/products/da9062
+ - https://www.dialog-semiconductor.com/products/da9061
properties:
compatible:
enum:
+ - dlg,da9061
+ - dlg,da9062
- dlg,da9063
- dlg,da9063l
@@ -35,6 +42,19 @@ properties:
"#interrupt-cells":
const: 2
+ gpio:
+ type: object
+ $ref: /schemas/gpio/gpio.yaml#
+ unevaluatedProperties: false
+ properties:
+ compatible:
+ const: dlg,da9062-gpio
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+
onkey:
$ref: /schemas/input/dlg,da9062-onkey.yaml
@@ -42,7 +62,7 @@ properties:
type: object
additionalProperties: false
patternProperties:
- "^(ldo([1-9]|1[01])|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged)$":
+ "^(ldo([1-9]|1[01])|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck[1-4])$":
$ref: /schemas/regulator/regulator.yaml
unevaluatedProperties: false
@@ -52,7 +72,12 @@ properties:
unevaluatedProperties: false
properties:
compatible:
- const: dlg,da9063-rtc
+ enum:
+ - dlg,da9063-rtc
+ - dlg,da9062-rtc
+
+ thermal:
+ $ref: /schemas/thermal/dlg,da9062-thermal.yaml
watchdog:
$ref: /schemas/watchdog/dlg,da9062-watchdog.yaml
@@ -60,8 +85,65 @@ properties:
required:
- compatible
- reg
- - interrupts
- - interrupt-controller
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - dlg,da9063
+ - dlg,da9063l
+ then:
+ properties:
+ thermal: false
+ gpio: false
+ gpio-controller: false
+ "#gpio-cells": false
+ regulators:
+ patternProperties:
+ "^buck[1-4]$": false
+ required:
+ - interrupts
+ - interrupt-controller
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - dlg,da9062
+ then:
+ properties:
+ regulators:
+ patternProperties:
+ "^(ldo([5-9]|10|11)|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged)$": false
+ required:
+ - gpio
+ - onkey
+ - rtc
+ - thermal
+ - watchdog
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - dlg,da9061
+ then:
+ properties:
+ gpio: false
+ gpio-controller: false
+ "#gpio-cells": false
+ regulators:
+ patternProperties:
+ "^(ldo([5-9]|10|11)|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck4)$": false
+ rtc: false
+ required:
+ - onkey
+ - thermal
+ - watchdog
additionalProperties: false
@@ -118,4 +200,98 @@ examples:
};
};
};
+
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/regulator/dlg,da9063-regulator.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pmic@58 {
+ compatible = "dlg,da9062";
+ reg = <0x58>;
+ #interrupt-cells = <2>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-controller;
+
+ gpio {
+ compatible = "dlg,da9062-gpio";
+ status = "disabled";
+ };
+
+ onkey {
+ compatible = "dlg,da9062-onkey";
+ };
+
+ regulators {
+ buck1 {
+ regulator-name = "vdd_arm";
+ regulator-min-microvolt = <925000>;
+ regulator-max-microvolt = <1380000>;
+ regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+ regulator-always-on;
+ };
+ buck2 {
+ regulator-name = "vdd_soc";
+ regulator-min-microvolt = <1150000>;
+ regulator-max-microvolt = <1380000>;
+ regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+ regulator-always-on;
+ };
+ buck3 {
+ regulator-name = "vdd_ddr3";
+ regulator-min-microvolt = <1500000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+ regulator-always-on;
+ };
+ buck4 {
+ regulator-name = "vdd_eth";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+ regulator-always-on;
+ };
+ ldo1 {
+ regulator-name = "vdd_snvs";
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-always-on;
+ };
+ ldo2 {
+ regulator-name = "vdd_high";
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-always-on;
+ };
+ ldo3 {
+ regulator-name = "vdd_eth_io";
+ regulator-min-microvolt = <2500000>;
+ regulator-max-microvolt = <2500000>;
+ };
+ ldo4 {
+ regulator-name = "vdd_emmc";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ };
+ };
+
+ rtc {
+ compatible = "dlg,da9062-rtc";
+ status = "disabled";
+ };
+
+ thermal {
+ compatible = "dlg,da9062-thermal";
+ status = "disabled";
+ };
+
+ watchdog {
+ compatible = "dlg,da9062-watchdog";
+ dlg,use-sw-pm;
+ };
+ };
+ };
...
diff --git a/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml b/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
index 206635f74850..e8b2cac41084 100644
--- a/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
@@ -11,7 +11,7 @@ maintainers:
description: |
This module is part of the DA9061/DA9062. For more details about entire
- DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
+ DA906{1,2} chips see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
Junction temperature thermal module uses an interrupt signal to identify
high THERMAL_TRIP_HOT temperatures for the PMIC device.
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v2] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: srinivas pandruvada @ 2023-12-04 17:50 UTC (permalink / raw)
To: Kai-Heng Feng, jikos, benjamin.tissoires
Cc: linux-pm, linux-pci, Jian Hui Lee, Even Xu, linux-input,
linux-kernel
In-Reply-To: <20231108121940.288005-1-kai.heng.feng@canonical.com>
Hi Kai,
Sorry for he delay in getting back on this. I have a question below:
On Wed, 2023-11-08 at 14:19 +0200, Kai-Heng Feng wrote:
> Since PCI core and ACPI core already handles PCI PME wake and GPE
> wake
> when the device has wakeup capability, use device_init_wakeup() to
> let
> them do the wakeup setting work.
>
> Also add a shutdown callback which uses pci_prepare_to_sleep() to let
> PCI and ACPI set OOB wakeup for S5.
>
> Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v2:
> Rebase on ("HID: intel-ish-hid: ipc: Disable and reenable ACPI GPE
> bit")
>
> drivers/hid/intel-ish-hid/ipc/pci-ish.c | 67 ++++++-----------------
> --
> 1 file changed, 15 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> index 710fda5f19e1..65e7eeb2fa64 100644
> --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> @@ -119,50 +119,6 @@ static inline bool ish_should_leave_d0i3(struct
> pci_dev *pdev)
> return !pm_resume_via_firmware() || pdev->device ==
> CHV_DEVICE_ID;
> }
>
> -static int enable_gpe(struct device *dev)
> -{
> -#ifdef CONFIG_ACPI
> - acpi_status acpi_sts;
> - struct acpi_device *adev;
> - struct acpi_device_wakeup *wakeup;
> -
> - adev = ACPI_COMPANION(dev);
> - if (!adev) {
> - dev_err(dev, "get acpi handle failed\n");
> - return -ENODEV;
> - }
> - wakeup = &adev->wakeup;
> -
> - /*
> - * Call acpi_disable_gpe(), so that reference count
> - * gpe_event_info->runtime_count doesn't overflow.
> - * When gpe_event_info->runtime_count = 0, the call
> - * to acpi_disable_gpe() simply return.
> - */
> - acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
> -
> - acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup-
> >gpe_number);
> - if (ACPI_FAILURE(acpi_sts)) {
> - dev_err(dev, "enable ose_gpe failed\n");
> - return -EIO;
> - }
> -
> - return 0;
> -#else
> - return -ENODEV;
> -#endif
> -}
> -
> -static void enable_pme_wake(struct pci_dev *pdev)
> -{
> - if ((pci_pme_capable(pdev, PCI_D0) ||
> - pci_pme_capable(pdev, PCI_D3hot) ||
> - pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev-
> >dev)) {
> - pci_pme_active(pdev, true);
> - dev_dbg(&pdev->dev, "ish ipc driver pme wake
> enabled\n");
> - }
> -}
> -
> /**
> * ish_probe() - PCI driver probe callback
> * @pdev: pci device
> @@ -233,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>
> /* Enable PME for EHL */
> if (pdev->device == EHL_Ax_DEVICE_ID)
> - enable_pme_wake(pdev);
> + device_init_wakeup(dev, true);
For apple to apple comparison, which path will call
https://elixir.bootlin.com/linux/latest/C/ident/__pci_enable_wake
which will call pci_pme_active()?
Thanks,
Srinivas
>
> ret = ish_init(ishtp);
> if (ret)
> @@ -256,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
> ish_device_disable(ishtp_dev);
> }
>
> +
> +/**
> + * ish_shutdown() - PCI driver shutdown callback
> + * @pdev: pci device
> + *
> + * This function sets up wakeup for S5
> + */
> +static void ish_shutdown(struct pci_dev *pdev)
> +{
> + if (pdev->device == EHL_Ax_DEVICE_ID)
> + pci_prepare_to_sleep(pdev);
> +}
> +
> static struct device __maybe_unused *ish_resume_device;
>
> /* 50ms to get resume response */
> @@ -378,13 +347,6 @@ static int __maybe_unused ish_resume(struct
> device *device)
> struct pci_dev *pdev = to_pci_dev(device);
> struct ishtp_device *dev = pci_get_drvdata(pdev);
>
> - /* add this to finish power flow for EHL */
> - if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
> - pci_set_power_state(pdev, PCI_D0);
> - enable_pme_wake(pdev);
> - dev_dbg(dev->devc, "set power state to D0 for
> ehl\n");
> - }
> -
> ish_resume_device = device;
> dev->resume_flag = 1;
>
> @@ -400,6 +362,7 @@ static struct pci_driver ish_driver = {
> .id_table = ish_pci_tbl,
> .probe = ish_probe,
> .remove = ish_remove,
> + .shutdown = ish_shutdown,
> .driver.pm = &ish_pm_ops,
> };
>
^ permalink raw reply
* Re: [PATCH] HID: nintendo: add support for nso controllers
From: Daniel Ogorchock @ 2023-12-04 17:52 UTC (permalink / raw)
To: Ryan McClelland; +Cc: linux-input, benjamin.tissoires, jikos
In-Reply-To: <20231204081721.19507-1-rymcclel@gmail.com>
Hi Ryan
On Mon, Dec 4, 2023 at 3:17 AM Ryan McClelland <rymcclel@gmail.com> wrote:
>
> This adds support for the nintendo switch online controllers which
> include the SNES, Genesis, and N64 Controllers.
>
> As each nso controller only implements a subset of what a pro
> controller can do. Each of these 'features' were broken up in to
> seperate functions which include right stick, left stick, imu, and
> dpad and depending on the controller type that it is, it will call
> the supported functions appropriately.
>
> Each controller now has a struct which maps the bit within the hid
> in report to a button.
>
> The name given to the device now comes directly from the hid
> device name rather than looking up a predefined string.
>
> Signed-off-by: Ryan McClelland <rymcclel@gmail.com>
> ---
> drivers/hid/Kconfig | 13 +-
> drivers/hid/hid-ids.h | 5 +-
> drivers/hid/hid-nintendo.c | 897 ++++++++++++++++++++++++++-----------
> 3 files changed, 649 insertions(+), 266 deletions(-)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 4ce74af79657..347c284fb27e 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -761,14 +761,15 @@ config HID_MULTITOUCH
> module will be called hid-multitouch.
>
> config HID_NINTENDO
> - tristate "Nintendo Joy-Con and Pro Controller support"
> + tristate "Nintendo Joy-Con, NSO, and Pro Controller support"
> depends on NEW_LEDS
> depends on LEDS_CLASS
> select POWER_SUPPLY
> help
> - Adds support for the Nintendo Switch Joy-Cons and Pro Controller.
> + Adds support for the Nintendo Switch Joy-Cons, NSO, Pro Controller.
> All controllers support bluetooth, and the Pro Controller also supports
> - its USB mode.
> + its USB mode. This also includes support for the Nintendo Switch Online
> + Controllers which include the Genesis, SNES, and N64 controllers.
>
> To compile this driver as a module, choose M here: the
> module will be called hid-nintendo.
> @@ -779,9 +780,9 @@ config NINTENDO_FF
> select INPUT_FF_MEMLESS
> help
> Say Y here if you have a Nintendo Switch controller and want to enable
> - force feedback support for it. This works for both joy-cons and the pro
> - controller. For the pro controller, both rumble motors can be controlled
> - individually.
> + force feedback support for it. This works for both joy-cons, the pro
> + controller, and the NSO N64 controller. For the pro controller, both
> + rumble motors can be controlled individually.
>
> config HID_NTI
> tristate "NTI keyboard adapters"
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index c6e4e0d1f214..a90aa3c31dd0 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -986,7 +986,10 @@
> #define USB_DEVICE_ID_NINTENDO_JOYCONL 0x2006
> #define USB_DEVICE_ID_NINTENDO_JOYCONR 0x2007
> #define USB_DEVICE_ID_NINTENDO_PROCON 0x2009
> -#define USB_DEVICE_ID_NINTENDO_CHRGGRIP 0x200E
> +#define USB_DEVICE_ID_NINTENDO_CHRGGRIP 0x200e
> +#define USB_DEVICE_ID_NINTENDO_SNESCON 0x2017
> +#define USB_DEVICE_ID_NINTENDO_GENCON 0x201e
> +#define USB_DEVICE_ID_NINTENDO_N64CON 0x2019
>
> #define USB_VENDOR_ID_NOVATEK 0x0603
> #define USB_DEVICE_ID_NOVATEK_PCT 0x0600
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 138f154fecef..47af111ef3a2 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -3,6 +3,9 @@
> * HID driver for Nintendo Switch Joy-Cons and Pro Controllers
> *
> * Copyright (c) 2019-2021 Daniel J. Ogorchock <djogorchock@gmail.com>
> + * Portions Copyright (c) 2020 Nadia Holmquist Pedersen <nadia@nhp.sh>
> + * Copyright (c) 2022 Emily Strickland <linux@emily.st>
> + * Copyright (c) 2023 Ryan McClelland <rymcclel@gmail.com>
> *
> * The following resources/projects were referenced for this driver:
> * https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
> @@ -17,6 +20,9 @@
> * This driver supports the Nintendo Switch Joy-Cons and Pro Controllers. The
> * Pro Controllers can either be used over USB or Bluetooth.
> *
> + * This driver also incorporates support for Nintendo Switch Online controllers
> + * for the NES, SNES, Sega Genesis, and N64.
> + *
> * The driver will retrieve the factory calibration info from the controllers,
> * so little to no user calibration should be required.
> *
> @@ -305,9 +311,14 @@ enum joycon_ctlr_state {
>
> /* Controller type received as part of device info */
> enum joycon_ctlr_type {
> - JOYCON_CTLR_TYPE_JCL = 0x01,
> - JOYCON_CTLR_TYPE_JCR = 0x02,
> - JOYCON_CTLR_TYPE_PRO = 0x03,
> + JOYCON_CTLR_TYPE_JCL = 0x01,
> + JOYCON_CTLR_TYPE_JCR = 0x02,
> + JOYCON_CTLR_TYPE_PRO = 0x03,
> + JOYCON_CTLR_TYPE_NESL = 0x09,
> + JOYCON_CTLR_TYPE_NESR = 0x0A,
> + JOYCON_CTLR_TYPE_SNES = 0x0B,
> + JOYCON_CTLR_TYPE_GEN = 0x0D,
> + JOYCON_CTLR_TYPE_N64 = 0x0C,
> };
>
> struct joycon_stick_cal {
> @@ -348,6 +359,137 @@ static const u32 JC_BTN_SL_L = BIT(21);
> static const u32 JC_BTN_L = BIT(22);
> static const u32 JC_BTN_ZL = BIT(23);
>
> +struct joycon_ctlr_button_mapping {
> + u32 code;
> + u32 bit;
> +};
> +
> +/*
> + * D-pad is configured as buttons for the left Joy-Con only!
> + */
> +static const struct joycon_ctlr_button_mapping left_joycon_button_mappings[] = {
> + { BTN_TL, JC_BTN_L, },
> + { BTN_TL2, JC_BTN_ZL, },
> + { BTN_SELECT, JC_BTN_MINUS, },
> + { BTN_THUMBL, JC_BTN_LSTICK, },
> + { BTN_DPAD_UP, JC_BTN_UP, },
> + { BTN_DPAD_DOWN, JC_BTN_DOWN, },
> + { BTN_DPAD_LEFT, JC_BTN_LEFT, },
> + { BTN_DPAD_RIGHT, JC_BTN_RIGHT, },
> + { BTN_Z, JC_BTN_CAP, },
> + { /* sentinel */ },
> +};
> +
> +/*
> + * The unused *right*-side triggers become the SL/SR triggers for the *left*
> + * Joy-Con, if and only if we're not using a charging grip.
> + */
> +static const struct joycon_ctlr_button_mapping left_joycon_s_button_mappings[] = {
> + { BTN_TR, JC_BTN_SL_L, },
> + { BTN_TR2, JC_BTN_SR_L, },
> + { /* sentinel */ },
> +};
> +
> +static const struct joycon_ctlr_button_mapping right_joycon_button_mappings[] = {
> + { BTN_EAST, JC_BTN_A, },
> + { BTN_SOUTH, JC_BTN_B, },
> + { BTN_NORTH, JC_BTN_X, },
> + { BTN_WEST, JC_BTN_Y, },
> + { BTN_TR, JC_BTN_R, },
> + { BTN_TR2, JC_BTN_ZR, },
> + { BTN_START, JC_BTN_PLUS, },
> + { BTN_THUMBR, JC_BTN_RSTICK, },
> + { BTN_MODE, JC_BTN_HOME, },
> + { /* sentinel */ },
> +};
> +
> +/*
> + * The unused *left*-side triggers become the SL/SR triggers for the *right*
> + * Joy-Con, if and only if we're not using a charging grip.
> + */
> +static const struct joycon_ctlr_button_mapping right_joycon_s_button_mappings[] = {
> + { BTN_TL, JC_BTN_SL_R, },
> + { BTN_TL2, JC_BTN_SR_R, },
> + { /* sentinel */ },
> +};
> +
> +static const struct joycon_ctlr_button_mapping procon_button_mappings[] = {
> + { BTN_EAST, JC_BTN_A, },
> + { BTN_SOUTH, JC_BTN_B, },
> + { BTN_NORTH, JC_BTN_X, },
> + { BTN_WEST, JC_BTN_Y, },
> + { BTN_TL, JC_BTN_L, },
> + { BTN_TR, JC_BTN_R, },
> + { BTN_TL2, JC_BTN_ZL, },
> + { BTN_TR2, JC_BTN_ZR, },
> + { BTN_SELECT, JC_BTN_MINUS, },
> + { BTN_START, JC_BTN_PLUS, },
> + { BTN_THUMBL, JC_BTN_LSTICK, },
> + { BTN_THUMBR, JC_BTN_RSTICK, },
> + { BTN_MODE, JC_BTN_HOME, },
> + { BTN_Z, JC_BTN_CAP, },
> + { /* sentinel */ },
> +};
> +
> +static const struct joycon_ctlr_button_mapping nescon_button_mappings[] = {
> + { BTN_SOUTH, JC_BTN_A, },
> + { BTN_EAST, JC_BTN_B, },
> + { BTN_TL, JC_BTN_L, },
> + { BTN_TR, JC_BTN_R, },
> + { BTN_SELECT, JC_BTN_MINUS, },
> + { BTN_START, JC_BTN_PLUS, },
> + { /* sentinel */ },
> +};
> +
> +static const struct joycon_ctlr_button_mapping snescon_button_mappings[] = {
> + { BTN_EAST, JC_BTN_A, },
> + { BTN_SOUTH, JC_BTN_B, },
> + { BTN_NORTH, JC_BTN_X, },
> + { BTN_WEST, JC_BTN_Y, },
> + { BTN_TL, JC_BTN_L, },
> + { BTN_TR, JC_BTN_R, },
> + { BTN_TL2, JC_BTN_ZL, },
> + { BTN_TR2, JC_BTN_ZR, },
> + { BTN_SELECT, JC_BTN_MINUS, },
> + { BTN_START, JC_BTN_PLUS, },
> + { /* sentinel */ },
> +};
> +
> +/*
> + * "A", "B", and "C" are mapped positionally, rather than by label (e.g., "A"
> + * gets assigned to BTN_EAST instead of BTN_A).
> + */
> +static const struct joycon_ctlr_button_mapping gencon_button_mappings[] = {
> + { BTN_SOUTH, JC_BTN_A, },
> + { BTN_EAST, JC_BTN_B, },
> + { BTN_WEST, JC_BTN_R, },
> + { BTN_SELECT, JC_BTN_ZR, },
> + { BTN_START, JC_BTN_PLUS, },
> + { BTN_MODE, JC_BTN_HOME, },
> + { BTN_Z, JC_BTN_CAP, },
> + { /* sentinel */ },
> +};
> +
> +/*
> + * N64's C buttons get assigned to d-pad directions and registered as buttons.
> + */
> +static const struct joycon_ctlr_button_mapping n64con_button_mappings[] = {
> + { BTN_A, JC_BTN_A, },
> + { BTN_B, JC_BTN_B, },
> + { BTN_TL2, JC_BTN_ZL, }, /* Z */
> + { BTN_TL, JC_BTN_L, },
> + { BTN_TR, JC_BTN_R, },
> + { BTN_TR2, JC_BTN_LSTICK, }, /* ZR */
> + { BTN_START, JC_BTN_PLUS, },
> + { BTN_FORWARD, JC_BTN_Y, }, /* C UP */
> + { BTN_BACK, JC_BTN_ZR, }, /* C DOWN */
> + { BTN_LEFT, JC_BTN_X, }, /* C LEFT */
> + { BTN_RIGHT, JC_BTN_MINUS, }, /* C RIGHT */
> + { BTN_MODE, JC_BTN_HOME, },
> + { BTN_Z, JC_BTN_CAP, },
> + { /* sentinel */ },
> +};
> +
> enum joycon_msg_type {
> JOYCON_MSG_TYPE_NONE,
> JOYCON_MSG_TYPE_USB,
> @@ -506,13 +648,182 @@ struct joycon_ctlr {
> /* Does this controller have inputs associated with left joycon? */
> #define jc_type_has_left(ctlr) \
> (ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCL || \
> - ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO)
> + ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO || \
> + ctlr->ctlr_type == JOYCON_CTLR_TYPE_N64)
>
> /* Does this controller have inputs associated with right joycon? */
> #define jc_type_has_right(ctlr) \
> (ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCR || \
> ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO)
>
> +
> +/*
> + * Controller device helpers
> + *
> + * These look at the device ID known to the HID subsystem to identify a device,
> + * but take caution: some NSO devices lie about themselves (NES Joy-Cons and
> + * Sega Genesis controller). See type helpers below.
> + *
> + * These helpers are most useful early during the HID probe or in conjunction
> + * with the capability helpers below.
> + */
> +static inline bool joycon_device_is_left_joycon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONL;
> +}
> +
> +static inline bool joycon_device_is_right_joycon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONR;
> +}
> +
> +static inline bool joycon_device_is_procon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_PROCON;
> +}
> +
> +static inline bool joycon_device_is_chrggrip(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_CHRGGRIP;
> +}
> +
> +static inline bool joycon_device_is_snescon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_SNESCON;
> +}
> +
> +static inline bool joycon_device_is_gencon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_GENCON;
> +}
> +
> +static inline bool joycon_device_is_n64con(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_N64CON;
> +}
> +
> +static inline bool joycon_device_has_usb(struct joycon_ctlr *ctlr)
> +{
> + return joycon_device_is_procon(ctlr) ||
> + joycon_device_is_chrggrip(ctlr) ||
> + joycon_device_is_snescon(ctlr) ||
> + joycon_device_is_gencon(ctlr) ||
> + joycon_device_is_n64con(ctlr);
> +}
> +
> +/*
> + * Controller type helpers
> + *
> + * These are slightly different than the device-ID-based helpers above. They are
> + * generally more reliable, since they can distinguish between, e.g., Genesis
> + * versus SNES, or NES Joy-Cons versus regular Switch Joy-Cons. They're most
> + * useful for reporting available inputs. For other kinds of distinctions, see
> + * the capability helpers below.
> + *
> + * They have two major drawbacks: (1) they're not available until after we set
> + * the reporting method and then request the device info; (2) they can't
> + * distinguish all controllers (like the Charging Grip from the Pro controller.)
> + */
> +static inline bool joycon_type_is_left_joycon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCL;
> +}
> +
> +static inline bool joycon_type_is_right_joycon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCR;
> +}
> +
> +static inline bool joycon_type_is_procon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO;
> +}
> +
> +static inline bool joycon_type_is_snescon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->ctlr_type == JOYCON_CTLR_TYPE_SNES;
> +}
> +
> +static inline bool joycon_type_is_gencon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->ctlr_type == JOYCON_CTLR_TYPE_GEN;
> +}
> +
> +static inline bool joycon_type_is_n64con(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->ctlr_type == JOYCON_CTLR_TYPE_N64;
> +}
> +
> +static inline bool joycon_type_is_left_nescon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->ctlr_type == JOYCON_CTLR_TYPE_NESL;
> +}
> +
> +static inline bool joycon_type_is_right_nescon(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->ctlr_type == JOYCON_CTLR_TYPE_NESR;
> +}
> +
> +static inline bool joycon_type_has_left_controls(struct joycon_ctlr *ctlr)
> +{
> + return joycon_type_is_left_joycon(ctlr) ||
> + joycon_type_is_procon(ctlr);
> +}
> +
> +static inline bool joycon_type_has_right_controls(struct joycon_ctlr *ctlr)
> +{
> + return joycon_type_is_right_joycon(ctlr) ||
> + joycon_type_is_procon(ctlr);
> +}
> +
> +static inline bool joycon_type_is_any_joycon(struct joycon_ctlr *ctlr)
> +{
> + return joycon_type_is_left_joycon(ctlr) ||
> + joycon_type_is_right_joycon(ctlr) ||
> + joycon_device_is_chrggrip(ctlr);
> +}
> +
> +static inline bool joycon_type_is_any_nescon(struct joycon_ctlr *ctlr)
> +{
> + return joycon_type_is_left_nescon(ctlr) ||
> + joycon_type_is_right_nescon(ctlr);
> +}
> +
> +/*
> + * Controller capability helpers
> + *
> + * These helpers combine the use of the helpers above to detect certain
> + * capabilities during initialization. They are always accurate but (since they
> + * use type helpers) cannot be used early in the HID probe.
> + */
> +static inline bool joycon_has_imu(struct joycon_ctlr *ctlr)
> +{
> + return joycon_device_is_chrggrip(ctlr) ||
> + joycon_type_is_any_joycon(ctlr) ||
> + joycon_type_is_procon(ctlr);
> +}
> +
> +static inline bool joycon_has_joysticks(struct joycon_ctlr *ctlr)
> +{
> + return joycon_device_is_chrggrip(ctlr) ||
> + joycon_type_is_any_joycon(ctlr) ||
> + joycon_type_is_procon(ctlr) ||
> + joycon_type_is_n64con(ctlr);
> +}
> +
> +static inline bool joycon_has_rumble(struct joycon_ctlr *ctlr)
> +{
> + return joycon_device_is_chrggrip(ctlr) ||
> + joycon_type_is_any_joycon(ctlr) ||
> + joycon_type_is_procon(ctlr) ||
> + joycon_type_is_n64con(ctlr);
> +}
> +
> +static inline bool joycon_using_usb(struct joycon_ctlr *ctlr)
> +{
> + return ctlr->hdev->bus == BUS_USB;
> +}
> +
> static int __joycon_hid_send(struct hid_device *hdev, u8 *data, size_t len)
> {
> u8 *buf;
> @@ -1283,15 +1594,10 @@ static void joycon_parse_imu_report(struct joycon_ctlr *ctlr,
> }
> }
>
> -static void joycon_parse_report(struct joycon_ctlr *ctlr,
> - struct joycon_input_report *rep)
> +static void joycon_handle_rumble_report(struct joycon_ctlr *ctlr, struct joycon_input_report *rep)
> {
> - struct input_dev *dev = ctlr->input;
> unsigned long flags;
> - u8 tmp;
> - u32 btns;
> unsigned long msecs = jiffies_to_msecs(jiffies);
> - unsigned long report_delta_ms = msecs - ctlr->last_input_report_msecs;
>
> spin_lock_irqsave(&ctlr->lock, flags);
> if (IS_ENABLED(CONFIG_NINTENDO_FF) && rep->vibrator_report &&
> @@ -1310,11 +1616,21 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
> queue_work(ctlr->rumble_queue, &ctlr->rumble_worker);
> }
>
> - /* Parse the battery status */
> + spin_unlock_irqrestore(&ctlr->lock, flags);
> +}
> +
> +static void joycon_parse_battery_status(struct joycon_ctlr *ctlr, struct joycon_input_report *rep)
> +{
> + u8 tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ctlr->lock, flags);
> +
> tmp = rep->bat_con;
> ctlr->host_powered = tmp & BIT(0);
> ctlr->battery_charging = tmp & BIT(4);
> tmp = tmp >> 5;
> +
> switch (tmp) {
> case 0: /* empty */
> ctlr->battery_capacity = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
> @@ -1336,102 +1652,121 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
> hid_warn(ctlr->hdev, "Invalid battery status\n");
> break;
> }
> +
> spin_unlock_irqrestore(&ctlr->lock, flags);
> +}
>
> - /* Parse the buttons and sticks */
> - btns = hid_field_extract(ctlr->hdev, rep->button_status, 0, 24);
> -
> - if (jc_type_has_left(ctlr)) {
> - u16 raw_x;
> - u16 raw_y;
> - s32 x;
> - s32 y;
> -
> - /* get raw stick values */
> - raw_x = hid_field_extract(ctlr->hdev, rep->left_stick, 0, 12);
> - raw_y = hid_field_extract(ctlr->hdev,
> - rep->left_stick + 1, 4, 12);
> - /* map the stick values */
> - x = joycon_map_stick_val(&ctlr->left_stick_cal_x, raw_x);
> - y = -joycon_map_stick_val(&ctlr->left_stick_cal_y, raw_y);
> - /* report sticks */
> - input_report_abs(dev, ABS_X, x);
> - input_report_abs(dev, ABS_Y, y);
> -
> - /* report buttons */
> - input_report_key(dev, BTN_TL, btns & JC_BTN_L);
> - input_report_key(dev, BTN_TL2, btns & JC_BTN_ZL);
> - input_report_key(dev, BTN_SELECT, btns & JC_BTN_MINUS);
> - input_report_key(dev, BTN_THUMBL, btns & JC_BTN_LSTICK);
> - input_report_key(dev, BTN_Z, btns & JC_BTN_CAP);
> -
> - if (jc_type_is_joycon(ctlr)) {
> - /* Report the S buttons as the non-existent triggers */
> - input_report_key(dev, BTN_TR, btns & JC_BTN_SL_L);
> - input_report_key(dev, BTN_TR2, btns & JC_BTN_SR_L);
> -
> - /* Report d-pad as digital buttons for the joy-cons */
> - input_report_key(dev, BTN_DPAD_DOWN,
> - btns & JC_BTN_DOWN);
> - input_report_key(dev, BTN_DPAD_UP, btns & JC_BTN_UP);
> - input_report_key(dev, BTN_DPAD_RIGHT,
> - btns & JC_BTN_RIGHT);
> - input_report_key(dev, BTN_DPAD_LEFT,
> - btns & JC_BTN_LEFT);
> - } else {
> - int hatx = 0;
> - int haty = 0;
> -
> - /* d-pad x */
> - if (btns & JC_BTN_LEFT)
> - hatx = -1;
> - else if (btns & JC_BTN_RIGHT)
> - hatx = 1;
> - input_report_abs(dev, ABS_HAT0X, hatx);
> -
> - /* d-pad y */
> - if (btns & JC_BTN_UP)
> - haty = -1;
> - else if (btns & JC_BTN_DOWN)
> - haty = 1;
> - input_report_abs(dev, ABS_HAT0Y, haty);
> - }
> - }
> - if (jc_type_has_right(ctlr)) {
> - u16 raw_x;
> - u16 raw_y;
> - s32 x;
> - s32 y;
> -
> - /* get raw stick values */
> - raw_x = hid_field_extract(ctlr->hdev, rep->right_stick, 0, 12);
> - raw_y = hid_field_extract(ctlr->hdev,
> - rep->right_stick + 1, 4, 12);
> - /* map stick values */
> - x = joycon_map_stick_val(&ctlr->right_stick_cal_x, raw_x);
> - y = -joycon_map_stick_val(&ctlr->right_stick_cal_y, raw_y);
> - /* report sticks */
> - input_report_abs(dev, ABS_RX, x);
> - input_report_abs(dev, ABS_RY, y);
> -
> - /* report buttons */
> - input_report_key(dev, BTN_TR, btns & JC_BTN_R);
> - input_report_key(dev, BTN_TR2, btns & JC_BTN_ZR);
> - if (jc_type_is_joycon(ctlr)) {
> - /* Report the S buttons as the non-existent triggers */
> - input_report_key(dev, BTN_TL, btns & JC_BTN_SL_R);
> - input_report_key(dev, BTN_TL2, btns & JC_BTN_SR_R);
> - }
> - input_report_key(dev, BTN_START, btns & JC_BTN_PLUS);
> - input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK);
> - input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME);
> - input_report_key(dev, BTN_WEST, btns & JC_BTN_Y);
> - input_report_key(dev, BTN_NORTH, btns & JC_BTN_X);
> - input_report_key(dev, BTN_EAST, btns & JC_BTN_A);
> - input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B);
> +static void joycon_report_left_stick(struct joycon_ctlr *ctlr,
> + struct joycon_input_report *rep)
> +{
> + u16 raw_x;
> + u16 raw_y;
> + s32 x;
> + s32 y;
> +
> + raw_x = hid_field_extract(ctlr->hdev, rep->left_stick, 0, 12);
> + raw_y = hid_field_extract(ctlr->hdev, rep->left_stick + 1, 4, 12);
> +
> + x = joycon_map_stick_val(&ctlr->left_stick_cal_x, raw_x);
> + y = -joycon_map_stick_val(&ctlr->left_stick_cal_y, raw_y);
> +
> + input_report_abs(ctlr->input, ABS_X, x);
> + input_report_abs(ctlr->input, ABS_Y, y);
> +}
> +
> +static void joycon_report_right_stick(struct joycon_ctlr *ctlr,
> + struct joycon_input_report *rep)
> +{
> + u16 raw_x;
> + u16 raw_y;
> + s32 x;
> + s32 y;
> +
> + raw_x = hid_field_extract(ctlr->hdev, rep->right_stick, 0, 12);
> + raw_y = hid_field_extract(ctlr->hdev, rep->right_stick + 1, 4, 12);
> +
> + x = joycon_map_stick_val(&ctlr->right_stick_cal_x, raw_x);
> + y = -joycon_map_stick_val(&ctlr->right_stick_cal_y, raw_y);
> +
> + input_report_abs(ctlr->input, ABS_RX, x);
> + input_report_abs(ctlr->input, ABS_RY, y);
> +}
> +
> +static void joycon_report_dpad(struct joycon_ctlr *ctlr,
> + struct joycon_input_report *rep)
> +{
> + int hatx = 0;
> + int haty = 0;
> + u32 btns = hid_field_extract(ctlr->hdev, rep->button_status, 0, 24);
> +
> + if (btns & JC_BTN_LEFT)
> + hatx = -1;
> + else if (btns & JC_BTN_RIGHT)
> + hatx = 1;
> +
> + if (btns & JC_BTN_UP)
> + haty = -1;
> + else if (btns & JC_BTN_DOWN)
> + haty = 1;
> +
> + input_report_abs(ctlr->input, ABS_HAT0X, hatx);
> + input_report_abs(ctlr->input, ABS_HAT0Y, haty);
> +}
> +
> +static void joycon_report_buttons(struct joycon_ctlr *ctlr,
> + struct joycon_input_report *rep,
> + const struct joycon_ctlr_button_mapping button_mappings[])
> +{
> + const struct joycon_ctlr_button_mapping *button;
> + u32 status = hid_field_extract(ctlr->hdev, rep->button_status, 0, 24);
> +
> + for (button = button_mappings; button->code; button++)
> + input_report_key(ctlr->input, button->code, status & button->bit);
> +}
> +
> +static void joycon_parse_report(struct joycon_ctlr *ctlr,
> + struct joycon_input_report *rep)
> +{
> + unsigned long flags;
> + unsigned long msecs = jiffies_to_msecs(jiffies);
> + unsigned long report_delta_ms = msecs - ctlr->last_input_report_msecs;
> +
> + if (joycon_has_rumble(ctlr))
> + joycon_handle_rumble_report(ctlr, rep);
> +
> + joycon_parse_battery_status(ctlr, rep);
> +
> + if (joycon_type_is_left_joycon(ctlr)) {
> + joycon_report_left_stick(ctlr, rep);
> + joycon_report_buttons(ctlr, rep, left_joycon_button_mappings);
> + if (!joycon_device_is_chrggrip(ctlr))
> + joycon_report_buttons(ctlr, rep, left_joycon_s_button_mappings);
> + } else if (joycon_type_is_right_joycon(ctlr)) {
> + joycon_report_right_stick(ctlr, rep);
> + joycon_report_buttons(ctlr, rep, right_joycon_button_mappings);
> + if (!joycon_device_is_chrggrip(ctlr))
> + joycon_report_buttons(ctlr, rep, right_joycon_s_button_mappings);
> + } else if (joycon_type_is_procon(ctlr)) {
> + joycon_report_left_stick(ctlr, rep);
> + joycon_report_right_stick(ctlr, rep);
> + joycon_report_dpad(ctlr, rep);
> + joycon_report_buttons(ctlr, rep, procon_button_mappings);
> + } else if (joycon_type_is_any_nescon(ctlr)) {
> + joycon_report_dpad(ctlr, rep);
> + joycon_report_buttons(ctlr, rep, nescon_button_mappings);
> + } else if (joycon_type_is_snescon(ctlr)) {
> + joycon_report_dpad(ctlr, rep);
> + joycon_report_buttons(ctlr, rep, snescon_button_mappings);
> + } else if (joycon_type_is_gencon(ctlr)) {
> + joycon_report_dpad(ctlr, rep);
> + joycon_report_buttons(ctlr, rep, gencon_button_mappings);
> + } else if (joycon_type_is_n64con(ctlr)) {
> + joycon_report_left_stick(ctlr, rep);
> + joycon_report_dpad(ctlr, rep);
> + joycon_report_buttons(ctlr, rep, n64con_button_mappings);
> }
>
> - input_sync(dev);
> + input_sync(ctlr->input);
>
> spin_lock_irqsave(&ctlr->lock, flags);
> ctlr->last_input_report_msecs = msecs;
> @@ -1471,7 +1806,7 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
> }
>
> /* parse IMU data if present */
> - if (rep->id == JC_INPUT_IMU_DATA)
> + if ((rep->id == JC_INPUT_IMU_DATA) && joycon_has_imu(ctlr))
> joycon_parse_imu_report(ctlr, rep);
> }
>
> @@ -1684,123 +2019,65 @@ static int joycon_play_effect(struct input_dev *dev, void *data,
> }
> #endif /* IS_ENABLED(CONFIG_NINTENDO_FF) */
>
> -static const unsigned int joycon_button_inputs_l[] = {
> - BTN_SELECT, BTN_Z, BTN_THUMBL,
> - BTN_TL, BTN_TL2,
> - 0 /* 0 signals end of array */
> -};
> -
> -static const unsigned int joycon_button_inputs_r[] = {
> - BTN_START, BTN_MODE, BTN_THUMBR,
> - BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST,
> - BTN_TR, BTN_TR2,
> - 0 /* 0 signals end of array */
> -};
> -
> -/* We report joy-con d-pad inputs as buttons and pro controller as a hat. */
> -static const unsigned int joycon_dpad_inputs_jc[] = {
> - BTN_DPAD_UP, BTN_DPAD_DOWN, BTN_DPAD_LEFT, BTN_DPAD_RIGHT,
> - 0 /* 0 signals end of array */
> -};
> -
> -static int joycon_input_create(struct joycon_ctlr *ctlr)
> +static void joycon_config_left_stick(struct input_dev *idev)
> {
> - struct hid_device *hdev;
> - const char *name;
> - const char *imu_name;
> - int ret;
> - int i;
> -
> - hdev = ctlr->hdev;
> + input_set_abs_params(idev,
> + ABS_X,
> + -JC_MAX_STICK_MAG,
> + JC_MAX_STICK_MAG,
> + JC_STICK_FUZZ,
> + JC_STICK_FLAT);
> + input_set_abs_params(idev,
> + ABS_Y,
> + -JC_MAX_STICK_MAG,
> + JC_MAX_STICK_MAG,
> + JC_STICK_FUZZ,
> + JC_STICK_FLAT);
> +}
>
> - switch (hdev->product) {
> - case USB_DEVICE_ID_NINTENDO_PROCON:
> - name = "Nintendo Switch Pro Controller";
> - imu_name = "Nintendo Switch Pro Controller IMU";
> - break;
> - case USB_DEVICE_ID_NINTENDO_CHRGGRIP:
> - if (jc_type_has_left(ctlr)) {
> - name = "Nintendo Switch Left Joy-Con (Grip)";
> - imu_name = "Nintendo Switch Left Joy-Con IMU (Grip)";
> - } else {
> - name = "Nintendo Switch Right Joy-Con (Grip)";
> - imu_name = "Nintendo Switch Right Joy-Con IMU (Grip)";
> - }
> - break;
> - case USB_DEVICE_ID_NINTENDO_JOYCONL:
> - name = "Nintendo Switch Left Joy-Con";
> - imu_name = "Nintendo Switch Left Joy-Con IMU";
> - break;
> - case USB_DEVICE_ID_NINTENDO_JOYCONR:
> - name = "Nintendo Switch Right Joy-Con";
> - imu_name = "Nintendo Switch Right Joy-Con IMU";
> - break;
> - default: /* Should be impossible */
> - hid_err(hdev, "Invalid hid product\n");
> - return -EINVAL;
> - }
> +static void joycon_config_right_stick(struct input_dev *idev)
> +{
> + input_set_abs_params(idev,
> + ABS_RX,
> + -JC_MAX_STICK_MAG,
> + JC_MAX_STICK_MAG,
> + JC_STICK_FUZZ,
> + JC_STICK_FLAT);
> + input_set_abs_params(idev,
> + ABS_RY,
> + -JC_MAX_STICK_MAG,
> + JC_MAX_STICK_MAG,
> + JC_STICK_FUZZ,
> + JC_STICK_FLAT);
> +}
>
> - ctlr->input = devm_input_allocate_device(&hdev->dev);
> - if (!ctlr->input)
> - return -ENOMEM;
> - ctlr->input->id.bustype = hdev->bus;
> - ctlr->input->id.vendor = hdev->vendor;
> - ctlr->input->id.product = hdev->product;
> - ctlr->input->id.version = hdev->version;
> - ctlr->input->uniq = ctlr->mac_addr_str;
> - ctlr->input->name = name;
> - ctlr->input->phys = hdev->phys;
> - input_set_drvdata(ctlr->input, ctlr);
> +static void joycon_config_dpad(struct input_dev *idev)
> +{
> + input_set_abs_params(idev,
> + ABS_HAT0X,
> + -JC_MAX_DPAD_MAG,
> + JC_MAX_DPAD_MAG,
> + JC_DPAD_FUZZ,
> + JC_DPAD_FLAT);
> + input_set_abs_params(idev,
> + ABS_HAT0Y,
> + -JC_MAX_DPAD_MAG,
> + JC_MAX_DPAD_MAG,
> + JC_DPAD_FUZZ,
> + JC_DPAD_FLAT);
> +}
>
> - /* set up sticks and buttons */
> - if (jc_type_has_left(ctlr)) {
> - input_set_abs_params(ctlr->input, ABS_X,
> - -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
> - JC_STICK_FUZZ, JC_STICK_FLAT);
> - input_set_abs_params(ctlr->input, ABS_Y,
> - -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
> - JC_STICK_FUZZ, JC_STICK_FLAT);
> -
> - for (i = 0; joycon_button_inputs_l[i] > 0; i++)
> - input_set_capability(ctlr->input, EV_KEY,
> - joycon_button_inputs_l[i]);
> -
> - /* configure d-pad differently for joy-con vs pro controller */
> - if (hdev->product != USB_DEVICE_ID_NINTENDO_PROCON) {
> - for (i = 0; joycon_dpad_inputs_jc[i] > 0; i++)
> - input_set_capability(ctlr->input, EV_KEY,
> - joycon_dpad_inputs_jc[i]);
> - } else {
> - input_set_abs_params(ctlr->input, ABS_HAT0X,
> - -JC_MAX_DPAD_MAG, JC_MAX_DPAD_MAG,
> - JC_DPAD_FUZZ, JC_DPAD_FLAT);
> - input_set_abs_params(ctlr->input, ABS_HAT0Y,
> - -JC_MAX_DPAD_MAG, JC_MAX_DPAD_MAG,
> - JC_DPAD_FUZZ, JC_DPAD_FLAT);
> - }
> - }
> - if (jc_type_has_right(ctlr)) {
> - input_set_abs_params(ctlr->input, ABS_RX,
> - -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
> - JC_STICK_FUZZ, JC_STICK_FLAT);
> - input_set_abs_params(ctlr->input, ABS_RY,
> - -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
> - JC_STICK_FUZZ, JC_STICK_FLAT);
> -
> - for (i = 0; joycon_button_inputs_r[i] > 0; i++)
> - input_set_capability(ctlr->input, EV_KEY,
> - joycon_button_inputs_r[i]);
> - }
> +static void joycon_config_buttons(struct input_dev *idev,
> + const struct joycon_ctlr_button_mapping button_mappings[])
> +{
> + const struct joycon_ctlr_button_mapping *button;
>
> - /* Let's report joy-con S triggers separately */
> - if (hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONL) {
> - input_set_capability(ctlr->input, EV_KEY, BTN_TR);
> - input_set_capability(ctlr->input, EV_KEY, BTN_TR2);
> - } else if (hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONR) {
> - input_set_capability(ctlr->input, EV_KEY, BTN_TL);
> - input_set_capability(ctlr->input, EV_KEY, BTN_TL2);
> - }
> + for (button = button_mappings; button->code; button++)
> + input_set_capability(idev, EV_KEY, button->code);
> +}
>
> +static void joycon_config_rumble(struct joycon_ctlr *ctlr)
> +{
> #if IS_ENABLED(CONFIG_NINTENDO_FF)
> /* set up rumble */
> input_set_capability(ctlr->input, EV_FF, FF_RUMBLE);
> @@ -1813,10 +2090,15 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
> joycon_set_rumble(ctlr, 0, 0, false);
> ctlr->rumble_msecs = jiffies_to_msecs(jiffies);
> #endif
> +}
>
> - ret = input_register_device(ctlr->input);
> - if (ret)
> - return ret;
> +static int joycon_imu_input_create(struct joycon_ctlr *ctlr)
> +{
> + struct hid_device *hdev;
> + const char *imu_name;
> + int ret;
> +
> + hdev = ctlr->hdev;
>
> /* configure the imu input device */
> ctlr->imu_input = devm_input_allocate_device(&hdev->dev);
> @@ -1828,8 +2110,14 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
> ctlr->imu_input->id.product = hdev->product;
> ctlr->imu_input->id.version = hdev->version;
> ctlr->imu_input->uniq = ctlr->mac_addr_str;
> - ctlr->imu_input->name = imu_name;
> ctlr->imu_input->phys = hdev->phys;
> +
> + imu_name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s (IMU)", ctlr->input->name);
> + if (!imu_name)
> + return -ENOMEM;
> +
> + ctlr->imu_input->name = imu_name;
> +
> input_set_drvdata(ctlr->imu_input, ctlr);
>
> /* configure imu axes */
> @@ -1871,6 +2159,71 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
> return 0;
> }
>
> +static int joycon_input_create(struct joycon_ctlr *ctlr)
> +{
> + struct hid_device *hdev;
> + int ret;
> +
> + hdev = ctlr->hdev;
> +
> + ctlr->input = devm_input_allocate_device(&hdev->dev);
> + if (!ctlr->input)
> + return -ENOMEM;
> + ctlr->input->id.bustype = hdev->bus;
> + ctlr->input->id.vendor = hdev->vendor;
> + ctlr->input->id.product = hdev->product;
> + ctlr->input->id.version = hdev->version;
> + ctlr->input->uniq = ctlr->mac_addr_str;
> + ctlr->input->name = hdev->name;
> + ctlr->input->phys = hdev->phys;
> + input_set_drvdata(ctlr->input, ctlr);
> +
> + ret = input_register_device(ctlr->input);
> + if (ret)
> + return ret;
> +
> + if (joycon_type_is_right_joycon(ctlr)) {
> + joycon_config_right_stick(ctlr->input);
> + joycon_config_buttons(ctlr->input, right_joycon_button_mappings);
> + if (!joycon_device_is_chrggrip(ctlr))
> + joycon_config_buttons(ctlr->input, right_joycon_s_button_mappings);
> + } else if (joycon_type_is_left_joycon(ctlr)) {
> + joycon_config_left_stick(ctlr->input);
> + joycon_config_buttons(ctlr->input, left_joycon_button_mappings);
> + if (!joycon_device_is_chrggrip(ctlr))
> + joycon_config_buttons(ctlr->input, left_joycon_s_button_mappings);
> + } else if (joycon_type_is_procon(ctlr)) {
> + joycon_config_left_stick(ctlr->input);
> + joycon_config_right_stick(ctlr->input);
> + joycon_config_dpad(ctlr->input);
> + joycon_config_buttons(ctlr->input, procon_button_mappings);
> + } else if (joycon_type_is_any_nescon(ctlr)) {
> + joycon_config_dpad(ctlr->input);
> + joycon_config_buttons(ctlr->input, nescon_button_mappings);
> + } else if (joycon_type_is_snescon(ctlr)) {
> + joycon_config_dpad(ctlr->input);
> + joycon_config_buttons(ctlr->input, snescon_button_mappings);
> + } else if (joycon_type_is_gencon(ctlr)) {
> + joycon_config_dpad(ctlr->input);
> + joycon_config_buttons(ctlr->input, gencon_button_mappings);
> + } else if (joycon_type_is_n64con(ctlr)) {
> + joycon_config_dpad(ctlr->input);
> + joycon_config_left_stick(ctlr->input);
> + joycon_config_buttons(ctlr->input, n64con_button_mappings);
> + }
> +
> + if (joycon_has_imu(ctlr)) {
> + ret = joycon_imu_input_create(ctlr);
> + if (ret)
> + return ret;
> + }
> +
> + if (joycon_has_rumble(ctlr))
> + joycon_config_rumble(ctlr);
> +
> + return 0;
> +}
> +
> /* Because the subcommand sets all the leds at once, the brightness argument is ignored */
> static int joycon_player_led_brightness_set(struct led_classdev *led,
> enum led_brightness brightness)
> @@ -2107,9 +2460,7 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> struct joycon_input_report *report;
>
> req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
> - mutex_lock(&ctlr->output_mutex);
> ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
> - mutex_unlock(&ctlr->output_mutex);
> if (ret) {
> hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
> return ret;
> @@ -2132,8 +2483,17 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> return -ENOMEM;
> hid_info(ctlr->hdev, "controller MAC = %s\n", ctlr->mac_addr_str);
>
> - /* Retrieve the type so we can distinguish for charging grip */
> + /*
> + * Retrieve the type so we can distinguish the controller type
> + * Unfortantly the hdev->product can't always be used due to a ?bug?
> + * with the NSO Genesis controller. Over USB, it will report the
> + * PID as 0x201E, but over bluetooth it will report the PID as 0x2017
> + * which is the same as the NSO SNES controller. This is different from
> + * the rest of the controllers which will report the same PID over USB
> + * and bluetooth.
> + */
> ctlr->ctlr_type = report->subcmd_reply.data[2];
> + hid_dbg(ctlr->hdev, "controller type = 0x%02X\n", ctlr->ctlr_type);
>
> return 0;
> }
> @@ -2145,8 +2505,7 @@ static int joycon_init(struct hid_device *hdev)
>
> mutex_lock(&ctlr->output_mutex);
> /* if handshake command fails, assume ble pro controller */
> - if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> - !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> + if (joycon_using_usb(ctlr) && !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> hid_dbg(hdev, "detected USB controller\n");
> /* set baudrate for improved latency */
> ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> @@ -2171,24 +2530,43 @@ static int joycon_init(struct hid_device *hdev)
> goto out_unlock;
> }
>
> - /* get controller calibration data, and parse it */
> - ret = joycon_request_calibration(ctlr);
> + /* needed to retrieve the controller type */
> + ret = joycon_read_info(ctlr);
> if (ret) {
> - /*
> - * We can function with default calibration, but it may be
> - * inaccurate. Provide a warning, and continue on.
> - */
> - hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> + hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
> + ret);
> + goto out_unlock;
> }
>
> - /* get IMU calibration data, and parse it */
> - ret = joycon_request_imu_calibration(ctlr);
> - if (ret) {
> - /*
> - * We can function with default calibration, but it may be
> - * inaccurate. Provide a warning, and continue on.
> - */
> - hid_warn(hdev, "Unable to read IMU calibration data\n");
> + if (joycon_has_joysticks(ctlr)) {
> + /* get controller calibration data, and parse it */
> + ret = joycon_request_calibration(ctlr);
> + if (ret) {
> + /*
> + * We can function with default calibration, but it may be
> + * inaccurate. Provide a warning, and continue on.
> + */
> + hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> + }
> + }
> +
> + if (joycon_has_imu(ctlr)) {
> + /* get IMU calibration data, and parse it */
> + ret = joycon_request_imu_calibration(ctlr);
> + if (ret) {
> + /*
> + * We can function with default calibration, but it may be
> + * inaccurate. Provide a warning, and continue on.
> + */
> + hid_warn(hdev, "Unable to read IMU calibration data\n");
> + }
> +
> + /* Enable the IMU */
> + ret = joycon_enable_imu(ctlr);
> + if (ret) {
> + hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> + goto out_unlock;
> + }
> }
>
> /* Set the reporting mode to 0x30, which is the full report mode */
> @@ -2198,18 +2576,13 @@ static int joycon_init(struct hid_device *hdev)
> goto out_unlock;
> }
>
> - /* Enable rumble */
> - ret = joycon_enable_rumble(ctlr);
> - if (ret) {
> - hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> - goto out_unlock;
> - }
> -
> - /* Enable the IMU */
> - ret = joycon_enable_imu(ctlr);
> - if (ret) {
> - hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> - goto out_unlock;
> + if (joycon_has_rumble(ctlr)) {
> + /* Enable rumble */
> + ret = joycon_enable_rumble(ctlr);
> + if (ret) {
> + hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> + goto out_unlock;
> + }
> }
>
> out_unlock:
> @@ -2354,13 +2727,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
> goto err_close;
> }
>
> - ret = joycon_read_info(ctlr);
> - if (ret) {
> - hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
> - ret);
> - goto err_close;
> - }
> -
> /* Initialize the leds */
> ret = joycon_leds_create(ctlr);
> if (ret) {
> @@ -2432,6 +2798,12 @@ static int nintendo_hid_resume(struct hid_device *hdev)
> static const struct hid_device_id nintendo_hid_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> USB_DEVICE_ID_NINTENDO_PROCON) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> + USB_DEVICE_ID_NINTENDO_SNESCON) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> + USB_DEVICE_ID_NINTENDO_GENCON) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> + USB_DEVICE_ID_NINTENDO_N64CON) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> USB_DEVICE_ID_NINTENDO_PROCON) },
> { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> @@ -2440,6 +2812,12 @@ static const struct hid_device_id nintendo_hid_devices[] = {
> USB_DEVICE_ID_NINTENDO_JOYCONL) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> USB_DEVICE_ID_NINTENDO_JOYCONR) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> + USB_DEVICE_ID_NINTENDO_SNESCON) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> + USB_DEVICE_ID_NINTENDO_GENCON) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> + USB_DEVICE_ID_NINTENDO_N64CON) },
> { }
> };
> MODULE_DEVICE_TABLE(hid, nintendo_hid_devices);
> @@ -2458,6 +2836,7 @@ static struct hid_driver nintendo_hid_driver = {
> module_hid_driver(nintendo_hid_driver);
>
> MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Ryan McClelland <rymcclel@gmail.com>");
> +MODULE_AUTHOR("Emily Strickland <linux@emily.st>");
> MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
> MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");
> -
> --
> 2.34.1
>
Thanks a lot for the patch. It's great to have support for the
assortment of Nintendo's switch controller variants in the driver.
The refactor of the button mappings and variant detection looks great to me.
I've been able to test successfully on all the HW I have (pro
controller, joycon left, joycon right, joycons in charging grip, N64
controller, and SNES controller). (I don't have the genesis and NES
controller variants to test with).
Reviewed-by: Daniel J. Ogorchock <djogorchock@gmail.com>
Tested-by: Daniel J. Ogorchock <djogorchock@gmail.com>
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v6 14/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
From: Mario Limonciello @ 2023-12-04 19:02 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires
Cc: Patil.Reddy, platform-driver-x86, linux-input
In-Reply-To: <20231204101548.1458499-15-Shyam-sundar.S-k@amd.com>
On 12/4/2023 04:15, Shyam Sundar S K wrote:
> From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>
> AMDSFH has information about the User presence information via the Human
> Presence Detection (HPD) sensor which is part of the AMD sensor fusion hub.
> Add PMF and AMDSFH interface to get this information.
>
> Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
> drivers/hid/amd-sfh-hid/amd_sfh_common.h | 5 ++
> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 14 ++++++
> .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 33 +++++++++++++
> .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 +
> drivers/platform/x86/amd/pmf/Kconfig | 1 +
> drivers/platform/x86/amd/pmf/spc.c | 22 +++++++++
> include/linux/amd-pmf-io.h | 46 +++++++++++++++++++
> 7 files changed, 122 insertions(+)
> create mode 100644 include/linux/amd-pmf-io.h
>
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> index 2643bb14fee2..cd57037bf217 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> @@ -37,6 +37,10 @@ struct amd_mp2_sensor_info {
> dma_addr_t dma_address;
> };
>
> +struct sfh_dev_status {
> + bool is_hpd_present;
> +};
> +
> struct amd_mp2_dev {
> struct pci_dev *pdev;
> struct amdtp_cl_data *cl_data;
> @@ -47,6 +51,7 @@ struct amd_mp2_dev {
> struct amd_input_data in_data;
> /* mp2 active control status */
> u32 mp2_acs;
> + struct sfh_dev_status dev_en;
> };
>
> struct amd_mp2_ops {
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> index e9c6413af24a..0351b0fd394a 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> @@ -73,6 +73,12 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
> int i, status;
>
> for (i = 0; i < cl_data->num_hid_devices; i++) {
> + switch (cl_data->sensor_idx[i]) {
> + case HPD_IDX:
> + privdata->dev_en.is_hpd_present = false;
> + break;
> + }
> +
> if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
> privdata->mp2_ops->stop(privdata, cl_data->sensor_idx[i]);
> status = amd_sfh_wait_for_response
> @@ -178,6 +184,11 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
> rc = amdtp_hid_probe(i, cl_data);
> if (rc)
> goto cleanup;
> + switch (cl_data->sensor_idx[i]) {
> + case HPD_IDX:
> + privdata->dev_en.is_hpd_present = true;
> + break;
> + }
> }
> dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
> cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
> @@ -259,6 +270,7 @@ static void amd_mp2_pci_remove(void *privdata)
> {
> struct amd_mp2_dev *mp2 = privdata;
>
> + sfh_deinit_emp2();
> amd_sfh_hid_client_deinit(privdata);
> mp2->mp2_ops->stop_all(mp2);
> pci_intx(mp2->pdev, false);
> @@ -311,12 +323,14 @@ int amd_sfh1_1_init(struct amd_mp2_dev *mp2)
>
> rc = amd_sfh_irq_init(mp2);
> if (rc) {
> + sfh_deinit_emp2();
> dev_err(dev, "amd_sfh_irq_init failed\n");
> return rc;
> }
>
> rc = amd_sfh1_1_hid_client_init(mp2);
> if (rc) {
> + sfh_deinit_emp2();
> dev_err(dev, "amd_sfh1_1_hid_client_init failed\n");
> return rc;
> }
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> index 4f81ef2d4f56..f8758fb70b1a 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> @@ -7,11 +7,14 @@
> *
> * Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> */
> +#include <linux/amd-pmf-io.h>
> #include <linux/io-64-nonatomic-lo-hi.h>
> #include <linux/iopoll.h>
>
> #include "amd_sfh_interface.h"
>
> +static struct amd_mp2_dev *emp2;
> +
> static int amd_sfh_wait_response(struct amd_mp2_dev *mp2, u8 sid, u32 cmd_id)
> {
> struct sfh_cmd_response cmd_resp;
> @@ -73,7 +76,37 @@ static struct amd_mp2_ops amd_sfh_ops = {
> .response = amd_sfh_wait_response,
> };
>
> +void sfh_deinit_emp2(void)
> +{
> + emp2 = NULL;
> +}
> +
> void sfh_interface_init(struct amd_mp2_dev *mp2)
> {
> mp2->mp2_ops = &amd_sfh_ops;
> + emp2 = mp2;
> +}
> +
> +static int amd_sfh_hpd_info(u8 *user_present)
> +{
> + struct hpd_status hpdstatus;
> +
> + if (!emp2 || !emp2->dev_en.is_hpd_present)
> + return -ENODEV;
> +
> + hpdstatus.val = readl(emp2->mmio + AMD_C2P_MSG(4));
> + *user_present = hpdstatus.shpd.presence;
It's an unlikely problem considering there is only one consumer for this
function but if amd_sfh_hpd_info() was called with NULL as an argument
this is a NULL pointer derefence.
So I think this function should have at the beginning:
if (!user_present)
return -EINVAL;
> + return 0;
> +}
> +
> +int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
> +{
> + if (sfh_info) {
> + switch (op) {
> + case MT_HPD:
> + return amd_sfh_hpd_info(&sfh_info->user_present);
> + }
> + }
> + return -EINVAL;
> }
> +EXPORT_SYMBOL_GPL(amd_get_sfh_info);
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> index 75267b0fec70..2c211d28764d 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> @@ -165,6 +165,7 @@ struct hpd_status {
> };
>
> void sfh_interface_init(struct amd_mp2_dev *mp2);
> +void sfh_deinit_emp2(void);
> void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
> int amd_sfh_float_to_int(u32 flt32_val);
> #endif
> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
> index f246252bddd8..f4fa8bd8bda8 100644
> --- a/drivers/platform/x86/amd/pmf/Kconfig
> +++ b/drivers/platform/x86/amd/pmf/Kconfig
> @@ -10,6 +10,7 @@ config AMD_PMF
> depends on AMD_NB
> select ACPI_PLATFORM_PROFILE
> depends on TEE && AMDTEE
> + depends on AMD_SFH_HID
> help
> This driver provides support for the AMD Platform Management Framework.
> The goal is to enhance end user experience by making AMD PCs smarter,
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index a0423942f771..5e769dcb075a 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -10,6 +10,7 @@
> */
>
> #include <acpi/button.h>
> +#include <linux/amd-pmf-io.h>
> #include <linux/power_supply.h>
> #include <linux/units.h>
> #include "pmf.h"
> @@ -44,6 +45,7 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
> dev_dbg(dev->dev, "Max C0 Residency: %u\n", in->ev_info.max_c0residency);
> dev_dbg(dev->dev, "GFX Busy: %u\n", in->ev_info.gfx_busy);
> dev_dbg(dev->dev, "LID State: %s\n", in->ev_info.lid_state ? "close" : "open");
> + dev_dbg(dev->dev, "User Presence: %s\n", in->ev_info.user_present ? "Present" : "Away");
> dev_dbg(dev->dev, "==== TA inputs END ====\n");
> }
> #else
> @@ -147,6 +149,25 @@ static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_
> return 0;
> }
>
> +static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> + struct amd_sfh_info sfh_info;
> +
> + /* get HPD data */
> + amd_get_sfh_info(&sfh_info, MT_HPD);
There is still an error handling miss here. amd_get_sfh_info() can
return error codes but they're discarded and sfh_info hasn't been
initialized to anything so this could be garbage going into the switch()
statement.
So can you explicitly check for errors on amd_get_sfh_info()?
> + switch (sfh_info.user_present) {
> + case SFH_NOT_DETECTED:
> + in->ev_info.user_present = 0xff; /* assume no sensors connected */
> + break;
> + case SFH_USER_PRESENT:
> + in->ev_info.user_present = 1;
> + break;
> + case SFH_USER_AWAY:
> + in->ev_info.user_present = 0;
> + break;
> + }
> +}
> +
> void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> {
> /* TA side lid open is 1 and close is 0, hence the ! here */
> @@ -155,4 +176,5 @@ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_tab
> amd_pmf_get_smu_info(dev, in);
> amd_pmf_get_battery_info(dev, in);
> amd_pmf_get_slider_info(dev, in);
> + amd_pmf_get_sensor_info(dev, in);
> }
> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
> new file mode 100644
> index 000000000000..5b6d29d36922
> --- /dev/null
> +++ b/include/linux/amd-pmf-io.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * AMD Platform Management Framework Interface
> + *
> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + * Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> + */
> +
> +#ifndef AMD_PMF_IO_H
> +#define AMD_PMF_IO_H
> +
> +#include <linux/types.h>
> +
> +/**
> + * enum sfh_message_type - Query the SFH message type
> + * @MT_HPD: Message ID to know the Human presence info from MP2 FW
> + */
> +enum sfh_message_type {
> + MT_HPD,
> +};
> +
> +/**
> + * enum sfh_hpd_info - Query the Human presence information
> + * @SFH_NOT_DETECTED: Check the HPD connection information from MP2 FW
> + * @SFH_USER_PRESENT: Check if the user is present from HPD sensor
> + * @SFH_USER_AWAY: Check if the user is away from HPD sensor
> + */
> +enum sfh_hpd_info {
> + SFH_NOT_DETECTED,
> + SFH_USER_PRESENT,
> + SFH_USER_AWAY,
> +};
> +
> +/**
> + * struct amd_sfh_info - get HPD sensor info from MP2 FW
> + * @user_present: Populates the user presence information
> + */
> +struct amd_sfh_info {
> + u8 user_present;
> +};
> +
> +int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);
> +#endif
^ permalink raw reply
* Re: [PATCH v6 13/15] HID: amd_sfh: rename float_to_int() to amd_sfh_float_to_int()
From: Mario Limonciello @ 2023-12-04 19:02 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires
Cc: Patil.Reddy, platform-driver-x86, linux-input
In-Reply-To: <20231204101548.1458499-14-Shyam-sundar.S-k@amd.com>
On 12/4/2023 04:15, Shyam Sundar S K wrote:
> From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>
> Current amd_sfh driver has float_to_int() to convert units from
> float to int. This is fine until this function gets called outside of
> the current scope of file.
>
> Add a prefix "amd_sfh" to float_to_int() so that function represents
> the driver name. This function will be called by multiple callers in the
> next patch.
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 28 ++++++++++---------
> .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 +
> 2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> index 8a037de08e92..33fbdde8aff0 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> @@ -132,7 +132,7 @@ static void get_common_inputs(struct common_input_property *common, int report_i
> common->event_type = HID_USAGE_SENSOR_EVENT_DATA_UPDATED_ENUM;
> }
>
> -static int float_to_int(u32 flt32_val)
> +int amd_sfh_float_to_int(u32 flt32_val)
> {
> int fraction, shift, mantissa, sign, exp, zeropre;
>
> @@ -201,9 +201,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
> OFFSET_SENSOR_DATA_DEFAULT;
> memcpy_fromio(&accel_data, sensoraddr, sizeof(struct sfh_accel_data));
> get_common_inputs(&acc_input.common_property, report_id);
> - acc_input.in_accel_x_value = float_to_int(accel_data.acceldata.x) / 100;
> - acc_input.in_accel_y_value = float_to_int(accel_data.acceldata.y) / 100;
> - acc_input.in_accel_z_value = float_to_int(accel_data.acceldata.z) / 100;
> + acc_input.in_accel_x_value = amd_sfh_float_to_int(accel_data.acceldata.x) / 100;
> + acc_input.in_accel_y_value = amd_sfh_float_to_int(accel_data.acceldata.y) / 100;
> + acc_input.in_accel_z_value = amd_sfh_float_to_int(accel_data.acceldata.z) / 100;
> memcpy(input_report, &acc_input, sizeof(acc_input));
> report_size = sizeof(acc_input);
> break;
> @@ -212,9 +212,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
> OFFSET_SENSOR_DATA_DEFAULT;
> memcpy_fromio(&gyro_data, sensoraddr, sizeof(struct sfh_gyro_data));
> get_common_inputs(&gyro_input.common_property, report_id);
> - gyro_input.in_angel_x_value = float_to_int(gyro_data.gyrodata.x) / 1000;
> - gyro_input.in_angel_y_value = float_to_int(gyro_data.gyrodata.y) / 1000;
> - gyro_input.in_angel_z_value = float_to_int(gyro_data.gyrodata.z) / 1000;
> + gyro_input.in_angel_x_value = amd_sfh_float_to_int(gyro_data.gyrodata.x) / 1000;
> + gyro_input.in_angel_y_value = amd_sfh_float_to_int(gyro_data.gyrodata.y) / 1000;
> + gyro_input.in_angel_z_value = amd_sfh_float_to_int(gyro_data.gyrodata.z) / 1000;
> memcpy(input_report, &gyro_input, sizeof(gyro_input));
> report_size = sizeof(gyro_input);
> break;
> @@ -223,9 +223,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
> OFFSET_SENSOR_DATA_DEFAULT;
> memcpy_fromio(&mag_data, sensoraddr, sizeof(struct sfh_mag_data));
> get_common_inputs(&magno_input.common_property, report_id);
> - magno_input.in_magno_x = float_to_int(mag_data.magdata.x) / 100;
> - magno_input.in_magno_y = float_to_int(mag_data.magdata.y) / 100;
> - magno_input.in_magno_z = float_to_int(mag_data.magdata.z) / 100;
> + magno_input.in_magno_x = amd_sfh_float_to_int(mag_data.magdata.x) / 100;
> + magno_input.in_magno_y = amd_sfh_float_to_int(mag_data.magdata.y) / 100;
> + magno_input.in_magno_z = amd_sfh_float_to_int(mag_data.magdata.z) / 100;
> magno_input.in_magno_accuracy = mag_data.accuracy / 100;
> memcpy(input_report, &magno_input, sizeof(magno_input));
> report_size = sizeof(magno_input);
> @@ -235,13 +235,15 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
> OFFSET_SENSOR_DATA_DEFAULT;
> memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
> get_common_inputs(&als_input.common_property, report_id);
> - als_input.illuminance_value = float_to_int(als_data.lux);
> + als_input.illuminance_value = amd_sfh_float_to_int(als_data.lux);
>
> memcpy_fromio(&binfo, mp2->vsbase, sizeof(struct sfh_base_info));
> if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2) {
> als_input.light_color_temp = als_data.light_color_temp;
> - als_input.chromaticity_x_value = float_to_int(als_data.chromaticity_x);
> - als_input.chromaticity_y_value = float_to_int(als_data.chromaticity_y);
> + als_input.chromaticity_x_value =
> + amd_sfh_float_to_int(als_data.chromaticity_x);
> + als_input.chromaticity_y_value =
> + amd_sfh_float_to_int(als_data.chromaticity_y);
> }
>
> report_size = sizeof(als_input);
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> index 656c3e95ef8c..75267b0fec70 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> @@ -166,4 +166,5 @@ struct hpd_status {
>
> void sfh_interface_init(struct amd_mp2_dev *mp2);
> void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
> +int amd_sfh_float_to_int(u32 flt32_val);
> #endif
^ permalink raw reply
* Re: [PATCH v6 06/15] platform/x86/amd/pmf: Add support to get inputs from other subsystems
From: Mario Limonciello @ 2023-12-04 19:03 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires
Cc: Patil.Reddy, platform-driver-x86, linux-input
In-Reply-To: <20231204101548.1458499-7-Shyam-sundar.S-k@amd.com>
On 12/4/2023 04:15, Shyam Sundar S K wrote:
> PMF driver sends changing inputs from each subystem to TA for evaluating
> the conditions in the policy binary.
>
> Add initial support of plumbing in the PMF driver for Smart PC to get
> information from other subsystems in the kernel.
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/platform/x86/amd/pmf/Makefile | 2 +-
> drivers/platform/x86/amd/pmf/pmf.h | 18 ++++
> drivers/platform/x86/amd/pmf/spc.c | 122 ++++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/tee-if.c | 3 +
> 4 files changed, 144 insertions(+), 1 deletion(-)
> create mode 100644 drivers/platform/x86/amd/pmf/spc.c
>
> diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
> index d2746ee7369f..6b26e48ce8ad 100644
> --- a/drivers/platform/x86/amd/pmf/Makefile
> +++ b/drivers/platform/x86/amd/pmf/Makefile
> @@ -7,4 +7,4 @@
> obj-$(CONFIG_AMD_PMF) += amd-pmf.o
> amd-pmf-objs := core.o acpi.o sps.o \
> auto-mode.o cnqf.o \
> - tee-if.o
> + tee-if.o spc.o
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 092be501d4d3..a4a73b845c09 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -150,6 +150,21 @@ struct smu_pmf_metrics {
> u16 infra_gfx_maxfreq; /* in MHz */
> u16 skin_temp; /* in centi-Celsius */
> u16 device_state;
> + u16 curtemp; /* in centi-Celsius */
> + u16 filter_alpha_value;
> + u16 avg_gfx_clkfrequency;
> + u16 avg_fclk_frequency;
> + u16 avg_gfx_activity;
> + u16 avg_socclk_frequency;
> + u16 avg_vclk_frequency;
> + u16 avg_vcn_activity;
> + u16 avg_dram_reads;
> + u16 avg_dram_writes;
> + u16 avg_socket_power;
> + u16 avg_core_power[2];
> + u16 avg_core_c0residency[16];
> + u16 spare1;
> + u32 metrics_counter;
> } __packed;
>
> enum amd_stt_skin_temp {
> @@ -601,4 +616,7 @@ extern const struct attribute_group cnqf_feature_attribute_group;
> int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
> int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
> +
> +/* Smart PC - TA interfaces */
> +void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in);
> #endif /* PMF_H */
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> new file mode 100644
> index 000000000000..351efcbe83c4
> --- /dev/null
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD Platform Management Framework Driver - Smart PC Capabilities
> + *
> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + * Patil Rajesh Reddy <Patil.Reddy@amd.com>
> + */
> +
> +#include <acpi/button.h>
> +#include <linux/power_supply.h>
> +#include <linux/units.h>
> +#include "pmf.h"
> +
> +static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> + u16 max, avg = 0;
> + int i;
> +
> + memset(dev->buf, 0, sizeof(dev->m_table));
> + amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
> + memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
> +
> + in->ev_info.socket_power = dev->m_table.apu_power + dev->m_table.dgpu_power;
> + in->ev_info.skin_temperature = dev->m_table.skin_temp;
> +
> + /* Get the avg and max C0 residency of all the cores */
> + max = dev->m_table.avg_core_c0residency[0];
> + for (i = 0; i < ARRAY_SIZE(dev->m_table.avg_core_c0residency); i++) {
> + avg += dev->m_table.avg_core_c0residency[i];
> + if (dev->m_table.avg_core_c0residency[i] > max)
> + max = dev->m_table.avg_core_c0residency[i];
> + }
> +
> + avg = DIV_ROUND_CLOSEST(avg, ARRAY_SIZE(dev->m_table.avg_core_c0residency));
> + in->ev_info.avg_c0residency = avg;
> + in->ev_info.max_c0residency = max;
> + in->ev_info.gfx_busy = dev->m_table.avg_gfx_activity;
> +}
> +
> +static const char * const pmf_battery_supply_name[] = {
> + "BATT",
> + "BAT0",
> +};
> +
> +static int amd_pmf_get_battery_prop(enum power_supply_property prop)
> +{
> + union power_supply_propval value;
> + struct power_supply *psy;
> + int i, ret;
> +
> + for (i = 0; i < ARRAY_SIZE(pmf_battery_supply_name); i++) {
> + psy = power_supply_get_by_name(pmf_battery_supply_name[i]);
> + if (!psy)
> + continue;
> +
> + ret = power_supply_get_property(psy, prop, &value);
> + if (ret) {
> + power_supply_put(psy);
> + return ret;
> + }
> + }
> +
> + return value.intval;
> +}
> +
> +static int amd_pmf_get_battery_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> + int val;
> +
> + val = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_PRESENT);
> + if (val < 0)
> + return val;
> + if (val != 1)
> + return -ENODEV;
> +
> + in->ev_info.bat_percentage = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_CAPACITY);
> + /* all values in mWh metrics */
> + in->ev_info.bat_design = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN) /
> + MILLIWATT_PER_WATT;
> + in->ev_info.full_charge_capacity = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL) /
> + MILLIWATT_PER_WATT;
> + in->ev_info.drain_rate = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_POWER_NOW) /
> + MILLIWATT_PER_WATT;
> +
> + return 0;
> +}
> +
> +static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> + int val;
> +
> + switch (dev->current_profile) {
> + case PLATFORM_PROFILE_PERFORMANCE:
> + val = TA_BEST_PERFORMANCE;
> + break;
> + case PLATFORM_PROFILE_BALANCED:
> + val = TA_BETTER_PERFORMANCE;
> + break;
> + case PLATFORM_PROFILE_LOW_POWER:
> + val = TA_BEST_BATTERY;
> + break;
> + default:
> + dev_err(dev->dev, "Unknown Platform Profile.\n");
> + return -EOPNOTSUPP;
> + }
> + in->ev_info.power_slider = val;
> +
> + return 0;
> +}
> +
> +void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> +{
> + /* TA side lid open is 1 and close is 0, hence the ! here */
> + in->ev_info.lid_state = !acpi_lid_open();
> + in->ev_info.power_source = amd_pmf_get_power_source();
> + amd_pmf_get_smu_info(dev, in);
> + amd_pmf_get_battery_info(dev, in);
> + amd_pmf_get_slider_info(dev, in);
> +}
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 468f3797a848..e3f17852d532 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -113,6 +113,7 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> {
> struct ta_pmf_shared_memory *ta_sm = NULL;
> struct ta_pmf_enact_result *out = NULL;
> + struct ta_pmf_enact_table *in = NULL;
> struct tee_param param[MAX_TEE_PARAM];
> struct tee_ioctl_invoke_arg arg;
> int ret = 0;
> @@ -123,11 +124,13 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> memset(dev->shbuf, 0, dev->policy_sz);
> ta_sm = dev->shbuf;
> out = &ta_sm->pmf_output.policy_apply_table;
> + in = &ta_sm->pmf_input.enact_table;
>
> memset(ta_sm, 0, sizeof(*ta_sm));
> ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES;
> ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
>
> + amd_pmf_populate_ta_inputs(dev, in);
> amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES, &arg, param);
>
> ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
^ permalink raw reply
* Re: [PATCH v6 04/15] platform/x86/amd/pmf: Add support for PMF Policy Binary
From: Mario Limonciello @ 2023-12-04 19:05 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires
Cc: Patil.Reddy, platform-driver-x86, linux-input
In-Reply-To: <20231204101548.1458499-5-Shyam-sundar.S-k@amd.com>
On 12/4/2023 04:15, Shyam Sundar S K wrote:
> PMF Policy binary is a encrypted and signed binary that will be part
> of the BIOS. PMF driver via the ACPI interface checks the existence
> of Smart PC bit. If the advertised bit is found, PMF driver walks
> the acpi namespace to find out the policy binary size and the address
> which has to be passed to the TA during the TA init sequence.
>
> The policy binary is comprised of inputs (or the events) and outputs
> (or the actions). With the PMF ecosystem, OEMs generate the policy
> binary (or could be multiple binaries) that contains a supported set
> of inputs and outputs which could be specifically carved out for each
> usage segment (or for each user also) that could influence the system
> behavior either by enriching the user experience or/and boost/throttle
> power limits.
>
> Once the TA init command succeeds, the PMF driver sends the changing
> events in the current environment to the TA for a constant sampling
> frequency time (the event here could be a lid close or open) and
> if the policy binary has corresponding action built within it, the
> TA sends the action for it in the subsequent enact command.
>
> If the inputs sent to the TA has no output defined in the policy
> binary generated by OEMs, there will be no action to be performed
> by the PMF driver.
>
> Example policies:
>
> 1) if slider is performance ; set the SPL to 40W
> Here PMF driver registers with the platform profile interface and
> when the slider position is changed, PMF driver lets the TA know
> about this. TA sends back an action to update the Sustained
> Power Limit (SPL). PMF driver updates this limit via the PMFW mailbox.
>
> 2) if user_away ; then lock the system
> Here PMF driver hooks to the AMD SFH driver to know the user presence
> and send the inputs to TA and if the condition is met, the TA sends
> the action of locking the system. PMF driver generates a uevent and
> based on the udev rule in the userland the system gets locked with
> systemctl.
>
> The intent here is to provide the OEM's to make a policy to lock the
> system when the user is away ; but the userland can make a choice to
> ignore it.
>
> and so on.
I think you can drop the 'and so on'.
>
> The OEMs will have an utility to create numerous such policies and
> the policies shall be reviewed by AMD before signing and encrypting
> them. Policies are shared between operating systems to have seemless user
> experience.
>
> Since all this action has to happen via the "amdtee" driver, currently
> there is no caller for it in the kernel which can load the amdtee driver.
> Without amdtee driver loading onto the system the "tee" calls shall fail
> from the PMF driver. Hence an explicit MODULE_SOFTDEP has been added
> to address this.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/platform/x86/amd/pmf/Kconfig | 2 +-
> drivers/platform/x86/amd/pmf/acpi.c | 37 +++++++
> drivers/platform/x86/amd/pmf/core.c | 9 ++
> drivers/platform/x86/amd/pmf/pmf.h | 141 ++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/tee-if.c | 147 +++++++++++++++++++++++++-
> 5 files changed, 333 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
> index 32a029e8db80..f246252bddd8 100644
> --- a/drivers/platform/x86/amd/pmf/Kconfig
> +++ b/drivers/platform/x86/amd/pmf/Kconfig
> @@ -9,7 +9,7 @@ config AMD_PMF
> depends on POWER_SUPPLY
> depends on AMD_NB
> select ACPI_PLATFORM_PROFILE
> - depends on TEE
> + depends on TEE && AMDTEE
> help
> This driver provides support for the AMD Platform Management Framework.
> The goal is to enhance end user experience by making AMD PCs smarter,
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 3fc5e4547d9f..4ec7957eb707 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -286,6 +286,43 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev)
> return 0;
> }
>
> +static acpi_status apmf_walk_resources(struct acpi_resource *res, void *data)
> +{
> + struct amd_pmf_dev *dev = data;
> +
> + switch (res->type) {
> + case ACPI_RESOURCE_TYPE_ADDRESS64:
> + dev->policy_addr = res->data.address64.address.minimum;
> + dev->policy_sz = res->data.address64.address.address_length;
> + break;
> + case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> + dev->policy_addr = res->data.fixed_memory32.address;
> + dev->policy_sz = res->data.fixed_memory32.address_length;
> + break;
> + }
> +
> + if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || dev->policy_sz == 0) {
> + pr_err("Incorrect Policy params, possibly a SBIOS bug\n");
> + return AE_ERROR;
> + }
> +
> + return AE_OK;
> +}
> +
> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev)
> +{
> + acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
> + acpi_status status;
> +
> + status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, apmf_walk_resources, pmf_dev);
> + if (ACPI_FAILURE(status)) {
> + dev_err(pmf_dev->dev, "acpi_walk_resources failed :%d\n", status);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev)
> {
> acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index f50b7ec9a625..c8f6ec4c2e2c 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -395,6 +395,14 @@ static int amd_pmf_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> dev->dev = &pdev->dev;
> + err = apmf_check_smart_pc(dev);
> + if (err)
> + /*
> + * Lets not return from here if Smart PC bit is not advertised in
> + * the BIOS. This way, there will be some amount of power savings
> + * to the user with static slider (if enabled).
> + */
> + pr_err("PMF Smart PC not advertised in BIOS!:%d\n", err);
>
> rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
> if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
> @@ -474,3 +482,4 @@ module_platform_driver(amd_pmf_driver);
>
> MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("AMD Platform Management Framework Driver");
> +MODULE_SOFTDEP("pre: amdtee");
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 6a0e4c446dd3..092be501d4d3 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -14,6 +14,11 @@
> #include <linux/acpi.h>
> #include <linux/platform_profile.h>
>
> +#define POLICY_BUF_MAX_SZ 0x4b000
> +#define POLICY_SIGN_COOKIE 0x31535024
> +#define POLICY_COOKIE_OFFSET 0x10
> +#define POLICY_COOKIE_LEN 0x14
> +
> /* APMF Functions */
> #define APMF_FUNC_VERIFY_INTERFACE 0
> #define APMF_FUNC_GET_SYS_PARAMS 1
> @@ -59,8 +64,21 @@
> #define ARG_NONE 0
> #define AVG_SAMPLE_SIZE 3
>
> +/* Policy Actions */
> +#define PMF_POLICY_SPL 2
> +#define PMF_POLICY_SPPT 3
> +#define PMF_POLICY_FPPT 4
> +#define PMF_POLICY_SPPT_APU_ONLY 5
> +#define PMF_POLICY_STT_MIN 6
> +#define PMF_POLICY_STT_SKINTEMP_APU 7
> +#define PMF_POLICY_STT_SKINTEMP_HS2 8
> +
> /* TA macros */
> #define PMF_TA_IF_VERSION_MAJOR 1
> +#define TA_PMF_ACTION_MAX 32
> +#define TA_PMF_UNDO_MAX 8
> +#define TA_OUTPUT_RESERVED_MEM 906
> +#define MAX_OPERATION_PARAMS 4
>
> /* AMD PMF BIOS interfaces */
> struct apmf_verify_interface {
> @@ -183,11 +201,16 @@ struct amd_pmf_dev {
> bool cnqf_supported;
> struct notifier_block pwr_src_notifier;
> /* Smart PC solution builder */
> + unsigned char *policy_buf;
> + u32 policy_sz;
> struct tee_context *tee_ctx;
> struct tee_shm *fw_shm_pool;
> u32 session_id;
> void *shbuf;
> struct delayed_work pb_work;
> + struct pmf_action_table *prev_data;
> + u64 policy_addr;
> + void *policy_base;
> bool smart_pc_enabled;
> };
>
> @@ -399,17 +422,134 @@ struct apmf_dyn_slider_output {
> struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
> } __packed;
>
> +enum smart_pc_status {
> + PMF_SMART_PC_ENABLED,
> + PMF_SMART_PC_DISABLED,
> +};
> +
> +/* Smart PC - TA internals */
> +enum ta_slider {
> + TA_BEST_BATTERY,
> + TA_BETTER_BATTERY,
> + TA_BETTER_PERFORMANCE,
> + TA_BEST_PERFORMANCE,
> + TA_MAX,
> +};
> +
> /* Command ids for TA communication */
> enum ta_pmf_command {
> TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE,
> TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES,
> };
>
> +enum ta_pmf_error_type {
> + TA_PMF_TYPE_SUCCESS,
> + TA_PMF_ERROR_TYPE_GENERIC,
> + TA_PMF_ERROR_TYPE_CRYPTO,
> + TA_PMF_ERROR_TYPE_CRYPTO_VALIDATE,
> + TA_PMF_ERROR_TYPE_CRYPTO_VERIFY_OEM,
> + TA_PMF_ERROR_TYPE_POLICY_BUILDER,
> + TA_PMF_ERROR_TYPE_PB_CONVERT,
> + TA_PMF_ERROR_TYPE_PB_SETUP,
> + TA_PMF_ERROR_TYPE_PB_ENACT,
> + TA_PMF_ERROR_TYPE_ASD_GET_DEVICE_INFO,
> + TA_PMF_ERROR_TYPE_ASD_GET_DEVICE_PCIE_INFO,
> + TA_PMF_ERROR_TYPE_SYS_DRV_FW_VALIDATION,
> + TA_PMF_ERROR_TYPE_MAX,
> +};
> +
> +struct pmf_action_table {
> + u32 spl; /* in mW */
> + u32 sppt; /* in mW */
> + u32 sppt_apuonly; /* in mW */
> + u32 fppt; /* in mW */
> + u32 stt_minlimit; /* in mW */
> + u32 stt_skintemp_apu; /* in C */
> + u32 stt_skintemp_hs2; /* in C */
> +};
> +
> +/* Input conditions */
> +struct ta_pmf_condition_info {
> + u32 power_source;
> + u32 bat_percentage;
> + u32 power_slider;
> + u32 lid_state;
> + bool user_present;
> + u32 rsvd1[2];
> + u32 monitor_count;
> + u32 rsvd2[2];
> + u32 bat_design;
> + u32 full_charge_capacity;
> + int drain_rate;
> + bool user_engaged;
> + u32 device_state;
> + u32 socket_power;
> + u32 skin_temperature;
> + u32 rsvd3[5];
> + u32 ambient_light;
> + u32 length;
> + u32 avg_c0residency;
> + u32 max_c0residency;
> + u32 s0i3_entry;
> + u32 gfx_busy;
> + u32 rsvd4[7];
> + bool camera_state;
> + u32 workload_type;
> + u32 display_type;
> + u32 display_state;
> + u32 rsvd5[150];
> +};
> +
> +struct ta_pmf_load_policy_table {
> + u32 table_size;
> + u8 table[POLICY_BUF_MAX_SZ];
> +};
> +
> +/* TA initialization params */
> +struct ta_pmf_init_table {
> + u32 frequency; /* SMU sampling frequency */
> + bool validate;
> + bool sku_check;
> + bool metadata_macrocheck;
> + struct ta_pmf_load_policy_table policies_table;
> +};
> +
> +/* Everything the TA needs to Enact Policies */
> +struct ta_pmf_enact_table {
> + struct ta_pmf_condition_info ev_info;
> + u32 name;
> +};
> +
> +struct ta_pmf_action {
> + u32 action_index;
> + u32 value;
> +};
> +
> +/* Output actions from TA */
> +struct ta_pmf_enact_result {
> + u32 actions_count;
> + struct ta_pmf_action actions_list[TA_PMF_ACTION_MAX];
> + u32 undo_count;
> + struct ta_pmf_action undo_list[TA_PMF_UNDO_MAX];
> +};
> +
> +union ta_pmf_input {
> + struct ta_pmf_enact_table enact_table;
> + struct ta_pmf_init_table init_table;
> +};
> +
> +union ta_pmf_output {
> + struct ta_pmf_enact_result policy_apply_table;
> + u32 rsvd[TA_OUTPUT_RESERVED_MEM];
> +};
> +
> struct ta_pmf_shared_memory {
> int command_id;
> int resp_id;
> u32 pmf_result;
> u32 if_version;
> + union ta_pmf_output pmf_output;
> + union ta_pmf_input pmf_input;
> };
>
> /* Core Layer */
> @@ -460,4 +600,5 @@ extern const struct attribute_group cnqf_feature_attribute_group;
> /* Smart PC builder Layer */
> int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
> #endif /* PMF_H */
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 4036f435f1e2..468f3797a848 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -42,9 +42,77 @@ static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
> param[0].u.memref.shm_offs = 0;
> }
>
> +static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
> +{
> + u32 val;
> + int idx;
> +
> + for (idx = 0; idx < out->actions_count; idx++) {
> + val = out->actions_list[idx].value;
> + switch (out->actions_list[idx].action_index) {
> + case PMF_POLICY_SPL:
> + if (dev->prev_data->spl != val) {
> + amd_pmf_send_cmd(dev, SET_SPL, false, val, NULL);
> + dev_dbg(dev->dev, "update SPL: %u\n", val);
> + dev->prev_data->spl = val;
> + }
> + break;
> +
> + case PMF_POLICY_SPPT:
> + if (dev->prev_data->sppt != val) {
> + amd_pmf_send_cmd(dev, SET_SPPT, false, val, NULL);
> + dev_dbg(dev->dev, "update SPPT: %u\n", val);
> + dev->prev_data->sppt = val;
> + }
> + break;
> +
> + case PMF_POLICY_FPPT:
> + if (dev->prev_data->fppt != val) {
> + amd_pmf_send_cmd(dev, SET_FPPT, false, val, NULL);
> + dev_dbg(dev->dev, "update FPPT: %u\n", val);
> + dev->prev_data->fppt = val;
> + }
> + break;
> +
> + case PMF_POLICY_SPPT_APU_ONLY:
> + if (dev->prev_data->sppt_apuonly != val) {
> + amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, val, NULL);
> + dev_dbg(dev->dev, "update SPPT_APU_ONLY: %u\n", val);
> + dev->prev_data->sppt_apuonly = val;
> + }
> + break;
> +
> + case PMF_POLICY_STT_MIN:
> + if (dev->prev_data->stt_minlimit != val) {
> + amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, val, NULL);
> + dev_dbg(dev->dev, "update STT_MIN: %u\n", val);
> + dev->prev_data->stt_minlimit = val;
> + }
> + break;
> +
> + case PMF_POLICY_STT_SKINTEMP_APU:
> + if (dev->prev_data->stt_skintemp_apu != val) {
> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false, val, NULL);
> + dev_dbg(dev->dev, "update STT_SKINTEMP_APU: %u\n", val);
> + dev->prev_data->stt_skintemp_apu = val;
> + }
> + break;
> +
> + case PMF_POLICY_STT_SKINTEMP_HS2:
> + if (dev->prev_data->stt_skintemp_hs2 != val) {
> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false, val, NULL);
> + dev_dbg(dev->dev, "update STT_SKINTEMP_HS2: %u\n", val);
> + dev->prev_data->stt_skintemp_hs2 = val;
> + }
> + break;
> + }
> + }
> +}
> +
> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> {
> struct ta_pmf_shared_memory *ta_sm = NULL;
> + struct ta_pmf_enact_result *out = NULL;
> struct tee_param param[MAX_TEE_PARAM];
> struct tee_ioctl_invoke_arg arg;
> int ret = 0;
> @@ -52,7 +120,10 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> if (!dev->tee_ctx)
> return -ENODEV;
>
> + memset(dev->shbuf, 0, dev->policy_sz);
> ta_sm = dev->shbuf;
> + out = &ta_sm->pmf_output.policy_apply_table;
> +
> memset(ta_sm, 0, sizeof(*ta_sm));
> ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES;
> ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
> @@ -65,6 +136,12 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> return ret;
> }
>
> + if (ta_sm->pmf_result == TA_PMF_TYPE_SUCCESS && out->actions_count) {
> + dev_dbg(dev->dev, "action count:%u result:%x\n", out->actions_count,
> + ta_sm->pmf_result);
> + amd_pmf_apply_policies(dev, out);
> + }
> +
> return 0;
> }
>
> @@ -72,6 +149,7 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
> {
> struct ta_pmf_shared_memory *ta_sm = NULL;
> struct tee_param param[MAX_TEE_PARAM];
> + struct ta_pmf_init_table *in = NULL;
> struct tee_ioctl_invoke_arg arg;
> int ret = 0;
>
> @@ -80,10 +158,21 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
> return -ENODEV;
> }
>
> + dev_dbg(dev->dev, "Policy Binary size: %u bytes\n", dev->policy_sz);
> + memset(dev->shbuf, 0, dev->policy_sz);
> ta_sm = dev->shbuf;
> + in = &ta_sm->pmf_input.init_table;
> +
> ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE;
> ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
>
> + in->metadata_macrocheck = false;
> + in->sku_check = false;
> + in->validate = true;
> + in->frequency = pb_actions_ms;
> + in->policies_table.table_size = dev->policy_sz;
> +
> + memcpy(in->policies_table.table, dev->policy_buf, dev->policy_sz);
> amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE, &arg, param);
>
> ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
> @@ -103,6 +192,52 @@ static void amd_pmf_invoke_cmd(struct work_struct *work)
> schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms));
> }
>
> +static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
> +{
> + u32 cookie, length;
> + int res;
> +
> + cookie = readl(dev->policy_buf + POLICY_COOKIE_OFFSET);
> + length = readl(dev->policy_buf + POLICY_COOKIE_LEN);
> +
> + if (cookie != POLICY_SIGN_COOKIE || !length)
> + return -EINVAL;
> +
> + /* Update the actual length */
> + dev->policy_sz = length + 512;
> + res = amd_pmf_invoke_cmd_init(dev);
> + if (res == TA_PMF_TYPE_SUCCESS) {
> + /* Now its safe to announce that smart pc is enabled */
> + dev->smart_pc_enabled = PMF_SMART_PC_ENABLED;
> + /*
> + * Start collecting the data from TA FW after a small delay
> + * or else, we might end up getting stale values.
> + */
> + schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms * 3));
> + } else {
> + dev_err(dev->dev, "ta invoke cmd init failed err: %x\n", res);
> + dev->smart_pc_enabled = PMF_SMART_PC_DISABLED;
> + return res;
> + }
> +
> + return 0;
> +}
> +
> +static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
> +{
> + dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
> + if (!dev->policy_buf)
> + return -ENOMEM;
> +
> + dev->policy_base = devm_ioremap(dev->dev, dev->policy_addr, dev->policy_sz);
> + if (!dev->policy_base)
> + return -ENOMEM;
> +
> + memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
> +
> + return amd_pmf_start_policy_engine(dev);
> +}
> +
> static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
> {
> return ver->impl_id == TEE_IMPL_ID_AMDTEE;
> @@ -146,7 +281,7 @@ static int amd_pmf_tee_init(struct amd_pmf_dev *dev)
> goto out_ctx;
> }
>
> - size = sizeof(struct ta_pmf_shared_memory);
> + size = sizeof(struct ta_pmf_shared_memory) + dev->policy_sz;
> dev->fw_shm_pool = tee_shm_alloc_kernel_buf(dev->tee_ctx, size);
> if (IS_ERR(dev->fw_shm_pool)) {
> dev_err(dev->dev, "Failed to alloc TEE shared memory\n");
> @@ -190,11 +325,19 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
> return ret;
>
> INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
> - return 0;
> + amd_pmf_set_dram_addr(dev);
> + amd_pmf_get_bios_buffer(dev);
> + dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
> + if (!dev->prev_data)
> + return -ENOMEM;
> +
> + return dev->smart_pc_enabled;
> }
>
> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
> {
> + kfree(dev->prev_data);
> + kfree(dev->policy_buf);
> cancel_delayed_work_sync(&dev->pb_work);
> amd_pmf_tee_deinit(dev);
> }
^ permalink raw reply
* [PATCH 0/4] Simplify ISHTP client interface
From: Even Xu @ 2023-12-05 1:50 UTC (permalink / raw)
To: jikos, srinivas.pandruvada, bleung, tzungbi, groeck, linux-input,
chrome-platform
Cc: Even Xu
There are three ISHTP clients, they have same ISHTP interface to
connect, reset and disconnect. Also there are multiple steps for
connect, reset and disconnect.
Simplify multiple steps by creating two helper functions. This will
simplify code flow and also avoid code duplication. Also this removes
memory allocation calls during reset, by using these helper functions.
No functional changes are expected with this series.
Even Xu (4):
HID: Intel-ish-hid: Ishtp: Add helper functions for client connection
HID: intel-ish-hid: ishtp-hid-client: use helper functions for
connection
HID: intel-ish-hid: ishtp-fw-loader: use helper functions for
connection
platform: chrome: cros_ec_ishtp: use helper functions for connection
drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 60 ++-------
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 63 ++-------
drivers/hid/intel-ish-hid/ishtp/client.c | 185 +++++++++++++++++++++++++--
drivers/platform/chrome/cros_ec_ishtp.c | 74 +++--------
include/linux/intel-ish-client-if.h | 3 +
5 files changed, 217 insertions(+), 168 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/4] HID: Intel-ish-hid: Ishtp: Add helper functions for client connection
From: Even Xu @ 2023-12-05 1:50 UTC (permalink / raw)
To: jikos, srinivas.pandruvada, bleung, tzungbi, groeck, linux-input,
chrome-platform
Cc: Even Xu
In-Reply-To: <1701741033-26222-1-git-send-email-even.xu@intel.com>
For every ishtp client driver during initialization state, the flow is:
1 - Allocate an ISHTP client instance
2 - Reserve a host id and link the client instance
3 - Search a firmware client using UUID and get related
client information
4 - Bind firmware client id to the ISHTP client instance
5 - Set the state the ISHTP client instance to CONNECTING
6 - Send connect request to firmware
7 - Register event callback for messages from the firmware
During deinitizalization state, the flow is:
9 - Set the state the ISHTP client instance to ISHTP_CL_DISCONNECTING
10 - Issue disconnect request to firmware
11 - Unlike the client instance
12 - Flush message queue
13 - Free ISHTP client instance
Step 2-7 are identical to the steps of client driver initialization
and driver reset flow, but reallocation of the RX/TX ring buffers
can be avoided in reset flow.
Also for step 9-12, they are identical to the steps of client driver
failure handling after connect request, driver reset flow and
driver removing.
So, add two helper functions to simplify client driver code.
ishtp_cl_establish_connection()
ishtp_cl_destroy_connection()
No functional changes are expected.
Signed-off-by: Even Xu <even.xu@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp/client.c | 185 +++++++++++++++++++++++++++++--
include/linux/intel-ish-client-if.h | 3 +
2 files changed, 177 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index 2d92fc1..82c907f 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -339,16 +339,17 @@ static bool ishtp_cl_is_other_connecting(struct ishtp_cl *cl)
}
/**
- * ishtp_cl_connect() - Send connect request to firmware
+ * ishtp_cl_connect_to_fw() - Send connect request to firmware
* @cl: client device instance
*
- * Send a connect request for a client to firmware. If successful it will
- * RX and TX ring buffers
+ * Send a connect request to the firmware and wait for firmware response.
+ * If there is successful connection response from the firmware, change
+ * client state to ISHTP_CL_CONNECTED, and bind client to related
+ * firmware client_id.
*
- * Return: 0 if successful connect response from the firmware and able
- * to bind and allocate ring buffers or error code on failure
+ * Return: 0 for success and error code on failure
*/
-int ishtp_cl_connect(struct ishtp_cl *cl)
+static int ishtp_cl_connect_to_fw(struct ishtp_cl *cl)
{
struct ishtp_device *dev;
int rets;
@@ -358,8 +359,6 @@ int ishtp_cl_connect(struct ishtp_cl *cl)
dev = cl->dev;
- dev->print_log(dev, "%s() current_state = %d\n", __func__, cl->state);
-
if (ishtp_cl_is_other_connecting(cl)) {
dev->print_log(dev, "%s() Busy\n", __func__);
return -EBUSY;
@@ -405,6 +404,38 @@ int ishtp_cl_connect(struct ishtp_cl *cl)
return rets;
}
+ return rets;
+}
+
+/**
+ * ishtp_cl_connect() - Build connection with firmware
+ * @cl: client device instance
+ *
+ * Call ishtp_cl_connect_to_fw() to connect and bind to firmware. If successful,
+ * allocate RX and TX ring buffers, and start flow control with firmware to
+ * start communication.
+ *
+ * Return: 0 if there is successful connection to the firmware, allocate
+ * ring buffers.
+ */
+int ishtp_cl_connect(struct ishtp_cl *cl)
+{
+ struct ishtp_device *dev;
+ int rets;
+
+ if (!cl || !cl->dev)
+ return -ENODEV;
+
+ dev = cl->dev;
+
+ dev->print_log(dev, "%s() current_state = %d\n", __func__, cl->state);
+
+ rets = ishtp_cl_connect_to_fw(cl);
+ if (rets) {
+ dev->print_log(dev, "%s() Connect to fw failed\n", __func__);
+ return rets;
+ }
+
rets = ishtp_cl_alloc_rx_ring(cl);
if (rets) {
dev->print_log(dev, "%s() Alloc RX ring failed\n", __func__);
@@ -422,16 +453,148 @@ int ishtp_cl_connect(struct ishtp_cl *cl)
return rets;
}
- /* Upon successful connection and allocation, emit flow-control */
+ /*
+ * Upon successful connection and allocation, start flow-control.
+ */
rets = ishtp_cl_read_start(cl);
- dev->print_log(dev, "%s() successful\n", __func__);
-
return rets;
}
EXPORT_SYMBOL(ishtp_cl_connect);
/**
+ * ishtp_cl_establish_connection() - Establish connection with the firmware
+ * @cl: client device instance
+ * @uuid: uuid of the client to search
+ * @tx_size: TX ring buffer size
+ * @rx_size: RX ring buffer size
+ * @reset: true if called for reset connection, otherwise for first connection
+ *
+ * This is a helper function for client driver to build connection with firmware.
+ * If it's first time connecting to the firmware, set reset to false, this
+ * function will link client to bus, find client id and send connect request to
+ * the firmware.
+ *
+ * If it's called for reset handler where client lost connection after
+ * firmware reset, set reset to true, this function will reinit client state and
+ * establish connection again. In this case, this function reuses current client
+ * structure and ring buffers to avoid allocation failure and memory fragments.
+ *
+ * Return: 0 for successful connection with the firmware,
+ * or error code on failure
+ */
+int ishtp_cl_establish_connection(struct ishtp_cl *cl, const guid_t *uuid,
+ int tx_size, int rx_size, bool reset)
+{
+ struct ishtp_device *dev;
+ struct ishtp_fw_client *fw_client;
+ int rets;
+
+ if (!cl || !cl->dev)
+ return -ENODEV;
+
+ dev = cl->dev;
+
+ ishtp_set_connection_state(cl, ISHTP_CL_INITIALIZING);
+
+ /* reinit ishtp_cl structure if call for reset */
+ if (reset) {
+ cl->host_client_id = 0;
+ cl->fw_client_id = 0;
+ cl->ishtp_flow_ctrl_creds = 0;
+ cl->out_flow_ctrl_creds = 0;
+
+ cl->last_tx_path = CL_TX_PATH_IPC;
+ cl->last_dma_acked = 1;
+ cl->last_dma_addr = NULL;
+ cl->last_ipc_acked = 1;
+
+ cl->sending = 0;
+ cl->err_send_msg = 0;
+ cl->err_send_fc = 0;
+
+ cl->send_msg_cnt_ipc = 0;
+ cl->send_msg_cnt_dma = 0;
+ cl->recv_msg_cnt_ipc = 0;
+ cl->recv_msg_cnt_dma = 0;
+ cl->recv_msg_num_frags = 0;
+ cl->ishtp_flow_ctrl_cnt = 0;
+ cl->out_flow_ctrl_cnt = 0;
+ }
+
+ /* link to bus */
+ rets = ishtp_cl_link(cl);
+ if (rets) {
+ dev->print_log(dev, "%s() ishtp_cl_link failed\n", __func__);
+ return rets;
+ }
+
+ /* find firmware client */
+ fw_client = ishtp_fw_cl_get_client(dev, uuid);
+ if (!fw_client) {
+ dev->print_log(dev,
+ "%s() ish client uuid not found\n", __func__);
+ return -ENOENT;
+ }
+
+ ishtp_set_tx_ring_size(cl, tx_size);
+ ishtp_set_rx_ring_size(cl, rx_size);
+
+ ishtp_cl_set_fw_client_id(cl, ishtp_get_fw_client_id(fw_client));
+
+ ishtp_set_connection_state(cl, ISHTP_CL_CONNECTING);
+
+ /*
+ * For reset case, not allocate tx/rx ring buffer which are already
+ * done in ishtp_cl_connect() during first connection.
+ */
+ if (reset) {
+ rets = ishtp_cl_connect_to_fw(cl);
+ if (!rets)
+ rets = ishtp_cl_read_start(cl);
+ else
+ dev->print_log(dev,
+ "%s() connect to fw failed\n", __func__);
+ } else {
+ rets = ishtp_cl_connect(cl);
+ }
+
+ return rets;
+}
+EXPORT_SYMBOL(ishtp_cl_establish_connection);
+
+/**
+ * ishtp_cl_destroy_connection() - Disconnect with the firmware
+ * @cl: client device instance
+ * @reset: true if called for firmware reset, false for normal disconnection
+ *
+ * This is a helper function for client driver to disconnect with firmware,
+ * unlink to bus and flush message queue.
+ */
+void ishtp_cl_destroy_connection(struct ishtp_cl *cl, bool reset)
+{
+ if (!cl)
+ return;
+
+ if (reset) {
+ /*
+ * For reset case, connection is already lost during fw reset.
+ * Just set state to DISCONNECTED is enough.
+ */
+ ishtp_set_connection_state(cl, ISHTP_CL_DISCONNECTED);
+ } else {
+ if (cl->state != ISHTP_CL_DISCONNECTED) {
+ ishtp_set_connection_state(cl, ISHTP_CL_DISCONNECTING);
+ ishtp_cl_disconnect(cl);
+ }
+ }
+
+ ishtp_cl_unlink(cl);
+ ishtp_cl_flush_queues(cl);
+}
+EXPORT_SYMBOL(ishtp_cl_destroy_connection);
+
+/**
* ishtp_cl_read_start() - Prepare to read client message
* @cl: client device instance
*
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index f45f133..7716226 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -94,6 +94,9 @@ int ishtp_cl_link(struct ishtp_cl *cl);
void ishtp_cl_unlink(struct ishtp_cl *cl);
int ishtp_cl_disconnect(struct ishtp_cl *cl);
int ishtp_cl_connect(struct ishtp_cl *cl);
+int ishtp_cl_establish_connection(struct ishtp_cl *cl, const guid_t *uuid,
+ int tx_size, int rx_size, bool reset);
+void ishtp_cl_destroy_connection(struct ishtp_cl *cl, bool reset);
int ishtp_cl_send(struct ishtp_cl *cl, uint8_t *buf, size_t length);
int ishtp_cl_flush_queues(struct ishtp_cl *cl);
int ishtp_cl_io_rb_recycle(struct ishtp_cl_rb *rb);
--
2.7.4
^ permalink raw reply related
* [PATCH 2/4] HID: intel-ish-hid: ishtp-hid-client: use helper functions for connection
From: Even Xu @ 2023-12-05 1:50 UTC (permalink / raw)
To: jikos, srinivas.pandruvada, bleung, tzungbi, groeck, linux-input,
chrome-platform
Cc: Even Xu
In-Reply-To: <1701741033-26222-1-git-send-email-even.xu@intel.com>
Use helper functions ishtp_cl_establish_connection() and
ishtp_cl_destroy_connection() to establish and destroy connection
respectively. These functions are used during initialization, reset and
deinitialization flows.
No functional changes are expected.
Signed-off-by: Even Xu <even.xu@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 63 ++++++----------------------
1 file changed, 13 insertions(+), 50 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index e3d70c5..fbd4f8e 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -639,47 +639,26 @@ static int ishtp_get_report_descriptor(struct ishtp_cl *hid_ishtp_cl,
*
* Return: 0 on success, non zero on error
*/
-static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
+static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, bool reset)
{
- struct ishtp_device *dev;
struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
- struct ishtp_fw_client *fw_client;
int i;
int rv;
dev_dbg(cl_data_to_dev(client_data), "%s\n", __func__);
hid_ishtp_trace(client_data, "%s reset flag: %d\n", __func__, reset);
- rv = ishtp_cl_link(hid_ishtp_cl);
- if (rv) {
- dev_err(cl_data_to_dev(client_data),
- "ishtp_cl_link failed\n");
- return -ENOMEM;
- }
-
client_data->init_done = 0;
- dev = ishtp_get_ishtp_device(hid_ishtp_cl);
-
- /* Connect to FW client */
- ishtp_set_tx_ring_size(hid_ishtp_cl, HID_CL_TX_RING_SIZE);
- ishtp_set_rx_ring_size(hid_ishtp_cl, HID_CL_RX_RING_SIZE);
-
- fw_client = ishtp_fw_cl_get_client(dev, &hid_ishtp_id_table[0].guid);
- if (!fw_client) {
- dev_err(cl_data_to_dev(client_data),
- "ish client uuid not found\n");
- return -ENOENT;
- }
- ishtp_cl_set_fw_client_id(hid_ishtp_cl,
- ishtp_get_fw_client_id(fw_client));
- ishtp_set_connection_state(hid_ishtp_cl, ISHTP_CL_CONNECTING);
-
- rv = ishtp_cl_connect(hid_ishtp_cl);
+ rv = ishtp_cl_establish_connection(hid_ishtp_cl,
+ &hid_ishtp_id_table[0].guid,
+ HID_CL_TX_RING_SIZE,
+ HID_CL_RX_RING_SIZE,
+ reset);
if (rv) {
dev_err(cl_data_to_dev(client_data),
"client connect fail\n");
- goto err_cl_unlink;
+ goto err_cl_disconnect;
}
hid_ishtp_trace(client_data, "%s client connected\n", __func__);
@@ -723,10 +702,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
return 0;
err_cl_disconnect:
- ishtp_set_connection_state(hid_ishtp_cl, ISHTP_CL_DISCONNECTING);
- ishtp_cl_disconnect(hid_ishtp_cl);
-err_cl_unlink:
- ishtp_cl_unlink(hid_ishtp_cl);
+ ishtp_cl_destroy_connection(hid_ishtp_cl, reset);
return rv;
}
@@ -738,8 +714,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
*/
static void hid_ishtp_cl_deinit(struct ishtp_cl *hid_ishtp_cl)
{
- ishtp_cl_unlink(hid_ishtp_cl);
- ishtp_cl_flush_queues(hid_ishtp_cl);
+ ishtp_cl_destroy_connection(hid_ishtp_cl, false);
/* disband and free all Tx and Rx client-level rings */
ishtp_cl_free(hid_ishtp_cl);
@@ -749,33 +724,23 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
{
struct ishtp_cl_data *client_data;
struct ishtp_cl *hid_ishtp_cl;
- struct ishtp_cl_device *cl_device;
int retry;
int rv;
client_data = container_of(work, struct ishtp_cl_data, work);
hid_ishtp_cl = client_data->hid_ishtp_cl;
- cl_device = client_data->cl_device;
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
hid_ishtp_cl);
dev_dbg(ishtp_device(client_data->cl_device), "%s\n", __func__);
- hid_ishtp_cl_deinit(hid_ishtp_cl);
-
- hid_ishtp_cl = ishtp_cl_allocate(cl_device);
- if (!hid_ishtp_cl)
- return;
-
- ishtp_set_drvdata(cl_device, hid_ishtp_cl);
- ishtp_set_client_data(hid_ishtp_cl, client_data);
- client_data->hid_ishtp_cl = hid_ishtp_cl;
+ ishtp_cl_destroy_connection(hid_ishtp_cl, true);
client_data->num_hid_devices = 0;
for (retry = 0; retry < 3; ++retry) {
- rv = hid_ishtp_cl_init(hid_ishtp_cl, 1);
+ rv = hid_ishtp_cl_init(hid_ishtp_cl, true);
if (!rv)
break;
dev_err(cl_data_to_dev(client_data), "Retry reset init\n");
@@ -841,7 +806,7 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
ishtp_hid_print_trace = ishtp_trace_callback(cl_device);
- rv = hid_ishtp_cl_init(hid_ishtp_cl, 0);
+ rv = hid_ishtp_cl_init(hid_ishtp_cl, false);
if (rv) {
ishtp_cl_free(hid_ishtp_cl);
return rv;
@@ -868,11 +833,9 @@ static void hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
hid_ishtp_cl);
dev_dbg(ishtp_device(cl_device), "%s\n", __func__);
- ishtp_set_connection_state(hid_ishtp_cl, ISHTP_CL_DISCONNECTING);
- ishtp_cl_disconnect(hid_ishtp_cl);
+ hid_ishtp_cl_deinit(hid_ishtp_cl);
ishtp_put_device(cl_device);
ishtp_hid_remove(client_data);
- hid_ishtp_cl_deinit(hid_ishtp_cl);
hid_ishtp_cl = NULL;
--
2.7.4
^ permalink raw reply related
* [PATCH 3/4] HID: intel-ish-hid: ishtp-fw-loader: use helper functions for connection
From: Even Xu @ 2023-12-05 1:50 UTC (permalink / raw)
To: jikos, srinivas.pandruvada, bleung, tzungbi, groeck, linux-input,
chrome-platform
Cc: Even Xu
In-Reply-To: <1701741033-26222-1-git-send-email-even.xu@intel.com>
Use helper functions ishtp_cl_establish_connection() and
ishtp_cl_destroy_connection() to establish and destroy connection
respectively. These functions are used during initialization, reset and
deinitialization flows.
No functional changes are expected.
Signed-off-by: Even Xu <even.xu@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 60 ++++++-----------------------
1 file changed, 12 insertions(+), 48 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
index 16aa030..e157863 100644
--- a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
+++ b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
@@ -840,43 +840,22 @@ static void load_fw_from_host_handler(struct work_struct *work)
*
* Return: 0 for success, negative error code for failure
*/
-static int loader_init(struct ishtp_cl *loader_ishtp_cl, int reset)
+static int loader_init(struct ishtp_cl *loader_ishtp_cl, bool reset)
{
int rv;
- struct ishtp_fw_client *fw_client;
struct ishtp_cl_data *client_data =
ishtp_get_client_data(loader_ishtp_cl);
dev_dbg(cl_data_to_dev(client_data), "reset flag: %d\n", reset);
- rv = ishtp_cl_link(loader_ishtp_cl);
- if (rv < 0) {
- dev_err(cl_data_to_dev(client_data), "ishtp_cl_link failed\n");
- return rv;
- }
-
- /* Connect to firmware client */
- ishtp_set_tx_ring_size(loader_ishtp_cl, LOADER_CL_TX_RING_SIZE);
- ishtp_set_rx_ring_size(loader_ishtp_cl, LOADER_CL_RX_RING_SIZE);
-
- fw_client =
- ishtp_fw_cl_get_client(ishtp_get_ishtp_device(loader_ishtp_cl),
- &loader_ishtp_id_table[0].guid);
- if (!fw_client) {
- dev_err(cl_data_to_dev(client_data),
- "ISH client uuid not found\n");
- rv = -ENOENT;
- goto err_cl_unlink;
- }
-
- ishtp_cl_set_fw_client_id(loader_ishtp_cl,
- ishtp_get_fw_client_id(fw_client));
- ishtp_set_connection_state(loader_ishtp_cl, ISHTP_CL_CONNECTING);
-
- rv = ishtp_cl_connect(loader_ishtp_cl);
+ rv = ishtp_cl_establish_connection(loader_ishtp_cl,
+ &loader_ishtp_id_table[0].guid,
+ LOADER_CL_TX_RING_SIZE,
+ LOADER_CL_RX_RING_SIZE,
+ reset);
if (rv < 0) {
dev_err(cl_data_to_dev(client_data), "Client connect fail\n");
- goto err_cl_unlink;
+ goto err_cl_disconnect;
}
dev_dbg(cl_data_to_dev(client_data), "Client connected\n");
@@ -885,17 +864,14 @@ static int loader_init(struct ishtp_cl *loader_ishtp_cl, int reset)
return 0;
-err_cl_unlink:
- ishtp_cl_unlink(loader_ishtp_cl);
+err_cl_disconnect:
+ ishtp_cl_destroy_connection(loader_ishtp_cl, reset);
return rv;
}
static void loader_deinit(struct ishtp_cl *loader_ishtp_cl)
{
- ishtp_set_connection_state(loader_ishtp_cl, ISHTP_CL_DISCONNECTING);
- ishtp_cl_disconnect(loader_ishtp_cl);
- ishtp_cl_unlink(loader_ishtp_cl);
- ishtp_cl_flush_queues(loader_ishtp_cl);
+ ishtp_cl_destroy_connection(loader_ishtp_cl, false);
/* Disband and free all Tx and Rx client-level rings */
ishtp_cl_free(loader_ishtp_cl);
@@ -914,19 +890,7 @@ static void reset_handler(struct work_struct *work)
loader_ishtp_cl = client_data->loader_ishtp_cl;
cl_device = client_data->cl_device;
- /* Unlink, flush queues & start again */
- ishtp_cl_unlink(loader_ishtp_cl);
- ishtp_cl_flush_queues(loader_ishtp_cl);
- ishtp_cl_free(loader_ishtp_cl);
-
- loader_ishtp_cl = ishtp_cl_allocate(cl_device);
- if (!loader_ishtp_cl)
- return;
-
- ishtp_set_drvdata(cl_device, loader_ishtp_cl);
- ishtp_set_client_data(loader_ishtp_cl, client_data);
- client_data->loader_ishtp_cl = loader_ishtp_cl;
- client_data->cl_device = cl_device;
+ ishtp_cl_destroy_connection(loader_ishtp_cl, true);
rv = loader_init(loader_ishtp_cl, 1);
if (rv < 0) {
@@ -974,7 +938,7 @@ static int loader_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
INIT_WORK(&client_data->work_fw_load,
load_fw_from_host_handler);
- rv = loader_init(loader_ishtp_cl, 0);
+ rv = loader_init(loader_ishtp_cl, false);
if (rv < 0) {
ishtp_cl_free(loader_ishtp_cl);
return rv;
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox