Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v5 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Kamel Bouhara @ 2023-12-11 12:14 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: <20231211121430.1689139-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 | 667 +++++++++++++++++++
 4 files changed, 681 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..4ade2d1adba0
--- /dev/null
+++ b/drivers/input/touchscreen/touchnetix_axiom.c
@@ -0,0 +1,667 @@
+// 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)
+{
+	u32 i;
+
+	/* 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 (ts->usage_table[i].id != usage)
+			continue;
+
+		if (page >= ts->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 ((ts->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;
+	struct i2c_msg msg[2];
+	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);
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = sizeof(cmd_header);
+	msg[0].buf = (u8 *)&cmd_header;
+
+	msg[1].addr = client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].len = len;
+	msg[1].buf = (char *)buf;
+
+	error = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+	if (error != ARRAY_SIZE(msg)) {
+		dev_err(&client->dev,
+			"Failed reading usage %#x page %#x, error=%d\n",
+			usage, page, error);
+		return -EIO;
+	}
+
+	usleep_range(250, 300);
+
+	return 0;
+}
+
+/*
+ * One of the main purposes for reading the usage table is to identify
+ * which usages reside at which target address.
+ * When performing subsequent reads or writes to AXIOM, the target address
+ * 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;
+	struct device *dev = ts->dev;
+	u16 crc_report;
+	u16 crc_calc;
+	int error;
+	char len;
+
+	error = axiom_i2c_read(ts->client, AXIOM_REPORT_USAGE_ID, 0, report_data,
+			       ts->max_report_len);
+	if (error)
+		return error;
+
+	len = (report_data[0] & AXIOM_COMMS_REPORT_LEN_MASK) << 1;
+	if (!len) {
+		dev_err(dev, "Zero length report discarded.\n");
+		return -ENODATA;
+	}
+
+	/* Validate the report CRC */
+	crc_report = (report_data[len - 1] << 8) | (report_data[len - 2]);
+	/* Length is in 16 bit words and remove the size of the CRC16 itself */
+	crc_calc = crc16(0, report_data, (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, error,
+					     "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 v7 1/4] pwm: Rename pwm_apply_state() to pwm_apply_might_sleep()
From: Uwe Kleine-König @ 2023-12-11 14:30 UTC (permalink / raw)
  To: Sean Young
  Cc: linux-media, linux-pwm, Ivaylo Dimitrov, Thierry Reding,
	Jonathan Corbet, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, David Airlie, Daniel Vetter,
	Javier Martinez Canillas, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Jean Delvare, Guenter Roeck,
	Support Opensource, Dmitry Torokhov, Pavel Machek, Lee Jones,
	Mauro Carvalho Chehab, Hans de Goede, Ilpo Järvinen,
	Liam Girdwood, Mark Brown, Daniel Thompson, Jingoo Han,
	Helge Deller, Jani Nikula, linux-doc, linux-kernel, intel-gfx,
	dri-devel, linux-hwmon, linux-input, linux-leds,
	platform-driver-x86, linux-arm-kernel, linux-fbdev
In-Reply-To: <f2762a5abc13be9ec05a45927bbd84e9da5bb41c.1702282807.git.sean@mess.org>

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

On Mon, Dec 11, 2023 at 08:24:52AM +0000, Sean Young wrote:
> In order to introduce a pwm api which can be used from atomic context,
> we will need two functions for applying pwm changes:
> 
> 	int pwm_apply_might_sleep(struct pwm *, struct pwm_state *);
> 	int pwm_apply_atomic(struct pwm *, struct pwm_state *);
> 
> This commit just deals with renaming pwm_apply_state(), a following
> commit will introduce the pwm_apply_atomic() function.
> 
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> # for input
> Acked-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Acked-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Sean Young <sean@mess.org>

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

^ permalink raw reply

* Re: [PATCH v6 03/15] platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
From: Shyam Sundar S K @ 2023-12-11 15:15 UTC (permalink / raw)
  To: Hans de Goede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input
In-Reply-To: <bae8cbbb-47d9-4e03-9445-8ca9b50576af@redhat.com>

Hi Hans,

On 12/11/2023 2:10 PM, Hans de Goede wrote:
> Hi,
> 
> On 12/4/23 11:15, Shyam Sundar S K wrote:
>> In the current code, the metrics table information was required only
>> for auto-mode or CnQF at a given time. Hence keeping the return type
>> of amd_pmf_set_dram_addr() as static made sense.
>>
>> But with the addition of Smart PC builder feature, the metrics table
>> information has to be shared by the Smart PC also and this feature
>> resides outside of core.c.
>>
>> To make amd_pmf_set_dram_addr() visible outside of core.c make it
>> as a non-static function and move the allocation of memory for
>> metrics table from amd_pmf_init_metrics_table() to amd_pmf_set_dram_addr()
>> as amd_pmf_set_dram_addr() is the common function to set the DRAM
>> address.
>>
>> Add a suspend handler that can free up the allocated memory for getting
>> the metrics table information.
>>
>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>>  drivers/platform/x86/amd/pmf/core.c | 42 ++++++++++++++++++++++-------
>>  drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>>  2 files changed, 34 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
>> index ec92d1cc0dac..f50b7ec9a625 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -251,29 +251,35 @@ static const struct pci_device_id pmf_pci_ids[] = {
>>  	{ }
>>  };
>>  
>> -static void amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
>> +int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
>>  {
>>  	u64 phys_addr;
>>  	u32 hi, low;
>>  
>> +	/* Get Metrics Table Address */
>> +	dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
>> +	if (!dev->buf)
>> +		return -ENOMEM;
>> +
>>  	phys_addr = virt_to_phys(dev->buf);
>>  	hi = phys_addr >> 32;
>>  	low = phys_addr & GENMASK(31, 0);
>>  
>>  	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
>>  	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
>> +
>> +	return 0;
>>  }
>>  
>>  int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
>>  {
>> -	/* Get Metrics Table Address */
>> -	dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
>> -	if (!dev->buf)
>> -		return -ENOMEM;
>> +	int ret;
>>  
>>  	INIT_DELAYED_WORK(&dev->work_buffer, amd_pmf_get_metrics);
>>  
>> -	amd_pmf_set_dram_addr(dev);
>> +	ret = amd_pmf_set_dram_addr(dev);
>> +	if (ret)
>> +		return ret;
>>  
>>  	/*
>>  	 * Start collecting the metrics data after a small delay
>> @@ -284,17 +290,35 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
>>  	return 0;
>>  }
>>  
>> +static int amd_pmf_suspend_handler(struct device *dev)
>> +{
>> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
>> +
>> +	/*
>> +	 * Free the buffer allocated for storing the metrics table
>> +	 * information, as will have to allocate it freshly after
>> +	 * resume.
>> +	 */
>> +	kfree(pdev->buf);
> 
> This seems quite risky. You are freeing the memory here,
> but the SET_DRAM_ADDR_HIGH and SET_DRAM_ADDR_LO registers
> still point to it, so the hw may still access it.
> 
> IMHO it would be better to add a "bool alloc_buffer"
> parameter to amd_pmf_set_dram_addr() and set that
> to true on init and false on resume.
> 

Ok. I have made this change.

> Also I guess that this DRAM buffer is used by the new
> smartpc mode and specifically by the command send by
> amd_pmf_invoke_cmd() work. In that case you really
> need to make sure to cancel the work before
> freeing the buffer and then re-submit the work
> on resume.

With some sanity tests, I don't think so this is required. pdev->buf
is only used to get the metrics table info. So, I am keeping only the
freeing part + alloc_buffer thing and not cancel/resubmit in the
suspend/resume.

If there are issues in the future because of not including
cancel/resubmit thing, can we work that as a bug fix later? Would that
be OK for you?

Thanks,
Shyam

> 
> Regards,
> 
> Hans
> 
> 
> 
>> +
>> +	return 0;
>> +}
>> +
>>  static int amd_pmf_resume_handler(struct device *dev)
>>  {
>>  	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
>> +	int ret;
>>  
>> -	if (pdev->buf)
>> -		amd_pmf_set_dram_addr(pdev);
>> +	if (pdev->buf) {
>> +		ret = amd_pmf_set_dram_addr(pdev);
>> +		if (ret)
>> +			return ret;
>> +	}
>>  
>>  	return 0;
>>  }
>>  
>> -static DEFINE_SIMPLE_DEV_PM_OPS(amd_pmf_pm, NULL, amd_pmf_resume_handler);
>> +static DEFINE_SIMPLE_DEV_PM_OPS(amd_pmf_pm, amd_pmf_suspend_handler, amd_pmf_resume_handler);
>>  
>>  static void amd_pmf_init_features(struct amd_pmf_dev *dev)
>>  {
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>> index a24e34e42032..6a0e4c446dd3 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -421,6 +421,7 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev);
>>  int amd_pmf_get_power_source(void);
>>  int apmf_install_handler(struct amd_pmf_dev *pmf_dev);
>>  int apmf_os_power_slider_update(struct amd_pmf_dev *dev, u8 flag);
>> +int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev);
>>  
>>  /* SPS Layer */
>>  int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);
> 

^ permalink raw reply

* Re: [PATCH v6 03/15] platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
From: Hans de Goede @ 2023-12-11 15:55 UTC (permalink / raw)
  To: Shyam Sundar S K, markgross, ilpo.jarvinen, basavaraj.natikar,
	jikos, benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input
In-Reply-To: <4af63850-0cb5-4deb-8dad-39dbb425916b@amd.com>

Hi Shyam,

On 12/11/23 16:15, Shyam Sundar S K wrote:
> Hi Hans,
> 
> On 12/11/2023 2:10 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 12/4/23 11:15, Shyam Sundar S K wrote:
>>> In the current code, the metrics table information was required only
>>> for auto-mode or CnQF at a given time. Hence keeping the return type
>>> of amd_pmf_set_dram_addr() as static made sense.
>>>
>>> But with the addition of Smart PC builder feature, the metrics table
>>> information has to be shared by the Smart PC also and this feature
>>> resides outside of core.c.
>>>
>>> To make amd_pmf_set_dram_addr() visible outside of core.c make it
>>> as a non-static function and move the allocation of memory for
>>> metrics table from amd_pmf_init_metrics_table() to amd_pmf_set_dram_addr()
>>> as amd_pmf_set_dram_addr() is the common function to set the DRAM
>>> address.
>>>
>>> Add a suspend handler that can free up the allocated memory for getting
>>> the metrics table information.
>>>
>>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>> ---
>>>  drivers/platform/x86/amd/pmf/core.c | 42 ++++++++++++++++++++++-------
>>>  drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>>>  2 files changed, 34 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
>>> index ec92d1cc0dac..f50b7ec9a625 100644
>>> --- a/drivers/platform/x86/amd/pmf/core.c
>>> +++ b/drivers/platform/x86/amd/pmf/core.c
>>> @@ -251,29 +251,35 @@ static const struct pci_device_id pmf_pci_ids[] = {
>>>  	{ }
>>>  };
>>>  
>>> -static void amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
>>> +int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
>>>  {
>>>  	u64 phys_addr;
>>>  	u32 hi, low;
>>>  
>>> +	/* Get Metrics Table Address */
>>> +	dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
>>> +	if (!dev->buf)
>>> +		return -ENOMEM;
>>> +
>>>  	phys_addr = virt_to_phys(dev->buf);
>>>  	hi = phys_addr >> 32;
>>>  	low = phys_addr & GENMASK(31, 0);
>>>  
>>>  	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
>>>  	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
>>> +
>>> +	return 0;
>>>  }
>>>  
>>>  int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
>>>  {
>>> -	/* Get Metrics Table Address */
>>> -	dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
>>> -	if (!dev->buf)
>>> -		return -ENOMEM;
>>> +	int ret;
>>>  
>>>  	INIT_DELAYED_WORK(&dev->work_buffer, amd_pmf_get_metrics);
>>>  
>>> -	amd_pmf_set_dram_addr(dev);
>>> +	ret = amd_pmf_set_dram_addr(dev);
>>> +	if (ret)
>>> +		return ret;
>>>  
>>>  	/*
>>>  	 * Start collecting the metrics data after a small delay
>>> @@ -284,17 +290,35 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
>>>  	return 0;
>>>  }
>>>  
>>> +static int amd_pmf_suspend_handler(struct device *dev)
>>> +{
>>> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
>>> +
>>> +	/*
>>> +	 * Free the buffer allocated for storing the metrics table
>>> +	 * information, as will have to allocate it freshly after
>>> +	 * resume.
>>> +	 */
>>> +	kfree(pdev->buf);
>>
>> This seems quite risky. You are freeing the memory here,
>> but the SET_DRAM_ADDR_HIGH and SET_DRAM_ADDR_LO registers
>> still point to it, so the hw may still access it.
>>
>> IMHO it would be better to add a "bool alloc_buffer"
>> parameter to amd_pmf_set_dram_addr() and set that
>> to true on init and false on resume.
>>
> 
> Ok. I have made this change.
> 
>> Also I guess that this DRAM buffer is used by the new
>> smartpc mode and specifically by the command send by
>> amd_pmf_invoke_cmd() work. In that case you really
>> need to make sure to cancel the work before
>> freeing the buffer and then re-submit the work
>> on resume.
> 
> With some sanity tests, I don't think so this is required. pdev->buf
> is only used to get the metrics table info. So, I am keeping only the
> freeing part + alloc_buffer thing and not cancel/resubmit in the
> suspend/resume.
> 
> If there are issues in the future because of not including
> cancel/resubmit thing, can we work that as a bug fix later? Would that
> be OK for you?

If you are sure that it is safe to keep the work running
after your suspend-handler has run and on resume to
have it running before your resume handler has run
then yes that is ok.

This seems unwise though, adding a sync kill of the
work + requeue is only a couple of lines of code.

And what about accessing other subsystems like the
AMD HID sensors, I guess the work item may do that
too? That esp. seems unwise to do after your
suspend-handler has run.

Even if you stop the work from your suspend handler
you may need to add some dev-links to ensure
that the PMF linux-device is suspended before
e.g. the AMD HID sensors. That can be done in
a follow up patch though.

Regards,

Hans




^ permalink raw reply

* Re: [PATCH] HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround
From: Mikhail Khvoinitsky @ 2023-12-11 16:17 UTC (permalink / raw)
  To: Yauhen Kharuzhy
  Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
	ValdikSS
In-Reply-To: <20231209182132.i3saw7kcjyykhizu@jekhomev>

Hello.

Well, that's unfortunate. As far as I can see, yes, TrackPoint
Keyboard II behaves similarly to 1st gen Trackpoint Keyboard (a.k.a.
Compact) but not quite. The best way would be to make the keyboard not
emulate scrolling at all but not sure if its possible. For cptkbd it
required patching firmware, it might be also the case for 2nd gen.

> Maybe we should map the wheel HID reports to REL_Y/REL_X in
> lenovo_input_mapping_tpIIkbd() to allow libinput to do its wheel emulation job?

It might work but since emulated wheel events aren't continuous (at
least this is the case with cptkbd), it doesn't make much sense in
comparison with the original implementation — just ignore the middle
button if scrolling.

I'll try to get the keyboard and check, but at the moment it seems
that the best thing to do is before stopping applying a workaround,
check that it's a Trackpoint Compact keyboard, not anything else. If I
won't be able to find a better solution shortly, I'll make a patch
which makes sure that it's only cptkbd before disabling the
workaround.

On Sat, 9 Dec 2023 at 20:21, Yauhen Kharuzhy <jekhor@gmail.com> wrote:
>
> On Sat, Dec 09, 2023 at 06:56:48PM +0200, Yauhen Kharuzhy wrote:
> > On Sat, Dec 09, 2023 at 02:50:16PM +0200, Yauhen Kharuzhy wrote:
> > > On Sun, Sep 24, 2023 at 01:58:30AM +0300, Mikhail Khvainitski wrote:
> > > > Built-in firmware of cptkbd handles scrolling by itself (when middle
> > > > button is pressed) but with issues: it does not support horizontal and
> > > > hi-res scrolling and upon middle button release it sends middle button
> > > > click even if there was a scrolling event. Commit 3cb5ff0220e3 ("HID:
> > > > lenovo: Hide middle-button press until release") workarounds last
> > > > issue but it's impossible to workaround scrolling-related issues
> > > > without firmware modification.
> > > >
> > > > Likely, Dennis Schneider has reverse engineered the firmware and
> > > > provided an instruction on how to patch it [1]. However,
> > > > aforementioned workaround prevents userspace (libinput) from knowing
> > > > exact moment when middle button has been pressed down and performing
> > > > "On-Button scrolling". This commit detects correctly-behaving patched
> > > > firmware if cursor movement events has been received during middle
> > > > button being pressed and stops applying workaround for this device.
> > > >
> > > > Link: https://hohlerde.org/rauch/en/elektronik/projekte/tpkbd-fix/ [1]
> > >
> > > This patch breaks a scrolling at my ThinkPad TrackPoint Keyboard II: it
> > > starts to report middle-button push/release events with scrolling events
> > > between. A support for this keyboard was added in
> > > 24401f291dcc4f2c18b9e2f65763cbaadc7a1528 "HID: lenovo: Add support for
> > > ThinkPad TrackPoint Keyboard II" commit.
> >
> > I figured this out.
> >
> > This keyboard can emit REL_Y/REL_X events between of middle-button
> > events (if user was moving a cursor and press middle button without of
> > stopping this), so this algorithm does a false-positive detection and switches
> > the workaround off like for patched firmware:
> >
> > Event: time 1702140625.854777, type 2 (EV_REL), code 1 (REL_Y), value 2
> > Event: time 1702140625.854777, -------------- SYN_REPORT ------------
> > Event: time 1702140625.870769, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 1
> > Event: time 1702140625.870769, -------------- SYN_REPORT ------------
> > Event: time 1702140625.870771, type 2 (EV_REL), code 1 (REL_Y), value 2
> > Event: time 1702140625.870771, -------------- SYN_REPORT ------------
> > Event: time 1702140625.970780, type 2 (EV_REL), code 8 (REL_WHEEL), value -1
> > Event: time 1702140625.970780, -------------- SYN_REPORT ------------
> > Event: time 1702140626.058800, type 2 (EV_REL), code 8 (REL_WHEEL), value -1
> > Event: time 1702140626.058800, -------------- SYN_REPORT ------------
> > Event: time 1702140630.462974, type 4 (EV_MSC), code 4 (MSC_SCAN), value ffa000fb
> > Event: time 1702140630.462974, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 0
> > Event: time 1702140630.462974, -------------- SYN_REPORT ------------
>
> Maybe we should map the wheel HID reports to REL_Y/REL_X in
> lenovo_input_mapping_tpIIkbd() to allow libinput to do its wheel emulation job?
> I tried this but I am not familiar with HID drivers and had no success.
>
>
> >
> >
> > >
> > > There is an evtest output below:
> > >
> > > Without of commit:
> > >
> > > Middle-button click:
> > > Event: time 1702122290.593300, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 1
> > > Event: time 1702122290.593300, -------------- SYN_REPORT ------------
> > > Event: time 1702122290.593312, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 0
> > > Event: time 1702122290.593312, -------------- SYN_REPORT ------------
> > >
> > > Vertical scrolling:
> > > Event: time 1702122300.441627, type 2 (EV_REL), code 8 (REL_WHEEL), value -1
> > > Event: time 1702122300.441627, -------------- SYN_REPORT ------------
> > > Event: time 1702122300.565663, type 2 (EV_REL), code 8 (REL_WHEEL), value -1
> > > Event: time 1702122300.565663, -------------- SYN_REPORT ------------
> > >
> > > Horizontal scrolling:
> > > Event: time 1702122307.845969, type 2 (EV_REL), code 6 (REL_HWHEEL), value -1
> > > Event: time 1702122307.845969, -------------- SYN_REPORT ------------
> > > Event: time 1702122307.981954, type 2 (EV_REL), code 6 (REL_HWHEEL), value -1
> > > Event: time 1702122307.981954, -------------- SYN_REPORT ------------
> > >
> > >
> > >
> > > After commit:
> > >
> > > Middle-button click:
> > > Event: time 1702125091.290045, type 4 (EV_MSC), code 4 (MSC_SCAN), value ffa000fb
> > > Event: time 1702125091.290045, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 1
> > > Event: time 1702125091.290045, -------------- SYN_REPORT ------------
> > > Event: time 1702125092.626118, type 4 (EV_MSC), code 4 (MSC_SCAN), value ffa000fb
> > > Event: time 1702125092.626118, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 0
> > > Event: time 1702125092.626118, -------------- SYN_REPORT ------------
> > >
> > >
> > > Vscroll:
> > > Event: time 1702125286.653639, type 4 (EV_MSC), code 4 (MSC_SCAN), value ffa000fb
> > > Event: time 1702125286.653639, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 1
> > > Event: time 1702125286.653639, -------------- SYN_REPORT ------------
> > > Event: time 1702125287.929689, type 2 (EV_REL), code 8 (REL_WHEEL), value -1
> > > Event: time 1702125287.929689, -------------- SYN_REPORT ------------
> > > Event: time 1702125288.037688, type 2 (EV_REL), code 8 (REL_WHEEL), value -1
> > > Event: time 1702125288.037688, -------------- SYN_REPORT ------------
> > > Event: time 1702125290.481787, type 4 (EV_MSC), code 4 (MSC_SCAN), value ffa000fb
> > > Event: time 1702125290.481787, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 0
> > > Event: time 1702125290.481787, -------------- SYN_REPORT ------------
> > >
> > > Hscroll:
> > > Event: time 1702125293.841920, type 4 (EV_MSC), code 4 (MSC_SCAN), value ffa000fb
> > > Event: time 1702125293.841920, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 1
> > > Event: time 1702125293.841920, -------------- SYN_REPORT ------------
> > > Event: time 1702125294.761952, type 2 (EV_REL), code 6 (REL_HWHEEL), value -1
> > > Event: time 1702125294.761952, -------------- SYN_REPORT ------------
> > > Event: time 1702125294.893967, type 2 (EV_REL), code 6 (REL_HWHEEL), value -1
> > > Event: time 1702125294.893967, -------------- SYN_REPORT ------------
> > > Event: time 1702125296.134006, type 4 (EV_MSC), code 4 (MSC_SCAN), value ffa000fb
> > > Event: time 1702125296.134006, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 0
> > > Event: time 1702125296.134006, -------------- SYN_REPORT ------------
> >
> > --
> > Yauhen Kharuzhy
>
> --
> Yauhen Kharuzhy

^ permalink raw reply

* Re: [PATCH v6 03/15] platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
From: Shyam Sundar S K @ 2023-12-11 16:35 UTC (permalink / raw)
  To: Hans de Goede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input
In-Reply-To: <c3a46dec-0f55-4abb-a5c1-39ba467a3108@redhat.com>

Hi Hans,

On 12/11/2023 9:25 PM, Hans de Goede wrote:
> Hi Shyam,
> 
> On 12/11/23 16:15, Shyam Sundar S K wrote:
>> Hi Hans,
>>
>> On 12/11/2023 2:10 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 12/4/23 11:15, Shyam Sundar S K wrote:
>>>> In the current code, the metrics table information was required only
>>>> for auto-mode or CnQF at a given time. Hence keeping the return type
>>>> of amd_pmf_set_dram_addr() as static made sense.
>>>>
>>>> But with the addition of Smart PC builder feature, the metrics table
>>>> information has to be shared by the Smart PC also and this feature
>>>> resides outside of core.c.
>>>>
>>>> To make amd_pmf_set_dram_addr() visible outside of core.c make it
>>>> as a non-static function and move the allocation of memory for
>>>> metrics table from amd_pmf_init_metrics_table() to amd_pmf_set_dram_addr()
>>>> as amd_pmf_set_dram_addr() is the common function to set the DRAM
>>>> address.
>>>>
>>>> Add a suspend handler that can free up the allocated memory for getting
>>>> the metrics table information.
>>>>
>>>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>> ---
>>>>  drivers/platform/x86/amd/pmf/core.c | 42 ++++++++++++++++++++++-------
>>>>  drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>>>>  2 files changed, 34 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
>>>> index ec92d1cc0dac..f50b7ec9a625 100644
>>>> --- a/drivers/platform/x86/amd/pmf/core.c
>>>> +++ b/drivers/platform/x86/amd/pmf/core.c
>>>> @@ -251,29 +251,35 @@ static const struct pci_device_id pmf_pci_ids[] = {
>>>>  	{ }
>>>>  };
>>>>  
>>>> -static void amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
>>>> +int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
>>>>  {
>>>>  	u64 phys_addr;
>>>>  	u32 hi, low;
>>>>  
>>>> +	/* Get Metrics Table Address */
>>>> +	dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
>>>> +	if (!dev->buf)
>>>> +		return -ENOMEM;
>>>> +
>>>>  	phys_addr = virt_to_phys(dev->buf);
>>>>  	hi = phys_addr >> 32;
>>>>  	low = phys_addr & GENMASK(31, 0);
>>>>  
>>>>  	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
>>>>  	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
>>>> +
>>>> +	return 0;
>>>>  }
>>>>  
>>>>  int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
>>>>  {
>>>> -	/* Get Metrics Table Address */
>>>> -	dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
>>>> -	if (!dev->buf)
>>>> -		return -ENOMEM;
>>>> +	int ret;
>>>>  
>>>>  	INIT_DELAYED_WORK(&dev->work_buffer, amd_pmf_get_metrics);
>>>>  
>>>> -	amd_pmf_set_dram_addr(dev);
>>>> +	ret = amd_pmf_set_dram_addr(dev);
>>>> +	if (ret)
>>>> +		return ret;
>>>>  
>>>>  	/*
>>>>  	 * Start collecting the metrics data after a small delay
>>>> @@ -284,17 +290,35 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> +static int amd_pmf_suspend_handler(struct device *dev)
>>>> +{
>>>> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
>>>> +
>>>> +	/*
>>>> +	 * Free the buffer allocated for storing the metrics table
>>>> +	 * information, as will have to allocate it freshly after
>>>> +	 * resume.
>>>> +	 */
>>>> +	kfree(pdev->buf);
>>>
>>> This seems quite risky. You are freeing the memory here,
>>> but the SET_DRAM_ADDR_HIGH and SET_DRAM_ADDR_LO registers
>>> still point to it, so the hw may still access it.
>>>
>>> IMHO it would be better to add a "bool alloc_buffer"
>>> parameter to amd_pmf_set_dram_addr() and set that
>>> to true on init and false on resume.
>>>
>>
>> Ok. I have made this change.
>>
>>> Also I guess that this DRAM buffer is used by the new
>>> smartpc mode and specifically by the command send by
>>> amd_pmf_invoke_cmd() work. In that case you really
>>> need to make sure to cancel the work before
>>> freeing the buffer and then re-submit the work
>>> on resume.
>>
>> With some sanity tests, I don't think so this is required. pdev->buf
>> is only used to get the metrics table info. So, I am keeping only the
>> freeing part + alloc_buffer thing and not cancel/resubmit in the
>> suspend/resume.
>>
>> If there are issues in the future because of not including
>> cancel/resubmit thing, can we work that as a bug fix later? Would that
>> be OK for you?
> 
> If you are sure that it is safe to keep the work running
> after your suspend-handler has run and on resume to
> have it running before your resume handler has run
> then yes that is ok.
> 
> This seems unwise though, adding a sync kill of the
> work + requeue is only a couple of lines of code.
> 
> And what about accessing other subsystems like the
> AMD HID sensors, I guess the work item may do that
> too? That esp. seems unwise to do after your
> suspend-handler has run.
> 
> Even if you stop the work from your suspend handler
> you may need to add some dev-links to ensure
> that the PMF linux-device is suspended before
> e.g. the AMD HID sensors. That can be done in
> a follow up patch though.
> 

Sure, thank you. Will address them separately as a followup patch later.

Thanks,
Shyam

> Regards,
> 
> Hans
> 
> 
> 

^ permalink raw reply

* [PATCH 0/3] Add polling support for DA9063 onkey driver
From: Biju Das @ 2023-12-11 16:57 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Biju Das, Support Opensource, linux-input, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc

On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ
populated by default. This patch series aims to add polling support
by parsing the polling interval from device tree and then detect
short key press and long press key.

Biju Das (3):
  Input: da9063 - Simplify obtaining OF match data
  Input: da9063 - Use dev_err_probe()
  Input: da9063 - Add polling support

 drivers/input/misc/da9063_onkey.c | 153 ++++++++++++++++++------------
 1 file changed, 92 insertions(+), 61 deletions(-)

-- 
2.25.1


^ permalink raw reply

* [PATCH 1/3] Input: da9063 - Simplify obtaining OF match data
From: Biju Das @ 2023-12-11 16:57 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Biju Das, Support Opensource, linux-input, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc
In-Reply-To: <20231211165708.161808-1-biju.das.jz@bp.renesas.com>

Simplify probe() by replacing of_match_node() for retrieving match data by
device_get_match_data().

Some minor cleanups:
 * Remove the trailing comma in the terminator entry for the OF
   table making code robust against (theoretical) misrebases or other
   similar things where the new entry goes _after_ the termination without
   the compiler noticing.
 * Move OF table near to the user.
 * Arrange variables in reverse xmas tree order in probe().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/input/misc/da9063_onkey.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
index 74808bae326a..9351ce0bb405 100644
--- a/drivers/input/misc/da9063_onkey.c
+++ b/drivers/input/misc/da9063_onkey.c
@@ -74,13 +74,6 @@ static const struct da906x_chip_config da9062_regs = {
 	.name = "da9062-onkey",
 };
 
-static const struct of_device_id da9063_compatible_reg_id_table[] = {
-	{ .compatible = "dlg,da9063-onkey", .data = &da9063_regs },
-	{ .compatible = "dlg,da9062-onkey", .data = &da9062_regs },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, da9063_compatible_reg_id_table);
-
 static void da9063_poll_on(struct work_struct *work)
 {
 	struct da9063_onkey *onkey = container_of(work,
@@ -187,14 +180,8 @@ static irqreturn_t da9063_onkey_irq_handler(int irq, void *data)
 static int da9063_onkey_probe(struct platform_device *pdev)
 {
 	struct da9063_onkey *onkey;
-	const struct of_device_id *match;
-	int irq;
 	int error;
-
-	match = of_match_node(da9063_compatible_reg_id_table,
-			      pdev->dev.of_node);
-	if (!match)
-		return -ENXIO;
+	int irq;
 
 	onkey = devm_kzalloc(&pdev->dev, sizeof(struct da9063_onkey),
 			     GFP_KERNEL);
@@ -203,7 +190,10 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	onkey->config = match->data;
+	onkey->config = device_get_match_data(&pdev->dev);
+	if (!onkey->config)
+		return -ENXIO;
+
 	onkey->dev = &pdev->dev;
 
 	onkey->regmap = dev_get_regmap(pdev->dev.parent, NULL);
@@ -270,6 +260,13 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id da9063_compatible_reg_id_table[] = {
+	{ .compatible = "dlg,da9063-onkey", .data = &da9063_regs },
+	{ .compatible = "dlg,da9062-onkey", .data = &da9062_regs },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, da9063_compatible_reg_id_table);
+
 static struct platform_driver da9063_onkey_driver = {
 	.probe	= da9063_onkey_probe,
 	.driver	= {
-- 
2.25.1


^ permalink raw reply related

* [PATCH 2/3] Input: da9063 - Use dev_err_probe()
From: Biju Das @ 2023-12-11 16:57 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Biju Das, Support Opensource, linux-input, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc
In-Reply-To: <20231211165708.161808-1-biju.das.jz@bp.renesas.com>

Replace dev_err()->dev_err_probe() to simpilfy probe().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/input/misc/da9063_onkey.c | 46 ++++++++++++-------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
index 9351ce0bb405..536220662b38 100644
--- a/drivers/input/misc/da9063_onkey.c
+++ b/drivers/input/misc/da9063_onkey.c
@@ -185,10 +185,9 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 
 	onkey = devm_kzalloc(&pdev->dev, sizeof(struct da9063_onkey),
 			     GFP_KERNEL);
-	if (!onkey) {
-		dev_err(&pdev->dev, "Failed to allocate memory.\n");
-		return -ENOMEM;
-	}
+	if (!onkey)
+		return dev_err_probe(&pdev->dev, -ENOMEM,
+				     "Failed to allocate memory.\n");
 
 	onkey->config = device_get_match_data(&pdev->dev);
 	if (!onkey->config)
@@ -197,19 +196,17 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 	onkey->dev = &pdev->dev;
 
 	onkey->regmap = dev_get_regmap(pdev->dev.parent, NULL);
-	if (!onkey->regmap) {
-		dev_err(&pdev->dev, "Parent regmap unavailable.\n");
-		return -ENXIO;
-	}
+	if (!onkey->regmap)
+		return dev_err_probe(&pdev->dev, -ENXIO,
+				     "Parent regmap unavailable.\n");
 
 	onkey->key_power = !of_property_read_bool(pdev->dev.of_node,
 						  "dlg,disable-key-power");
 
 	onkey->input = devm_input_allocate_device(&pdev->dev);
-	if (!onkey->input) {
-		dev_err(&pdev->dev, "Failed to allocated input device.\n");
-		return -ENOMEM;
-	}
+	if (!onkey->input)
+		return dev_err_probe(&pdev->dev, -ENOMEM,
+				     "Failed to allocated input device.\n");
 
 	onkey->input->name = onkey->config->name;
 	snprintf(onkey->phys, sizeof(onkey->phys), "%s/input0",
@@ -221,12 +218,9 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 
 	error = devm_delayed_work_autocancel(&pdev->dev, &onkey->work,
 					     da9063_poll_on);
-	if (error) {
-		dev_err(&pdev->dev,
-			"Failed to add cancel poll action: %d\n",
-			error);
-		return error;
-	}
+	if (error)
+		return dev_err_probe(&pdev->dev, error,
+				     "Failed to add cancel poll action\n");
 
 	irq = platform_get_irq_byname(pdev, "ONKEY");
 	if (irq < 0)
@@ -236,11 +230,9 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 					  NULL, da9063_onkey_irq_handler,
 					  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 					  "ONKEY", onkey);
-	if (error) {
-		dev_err(&pdev->dev,
-			"Failed to request IRQ %d: %d\n", irq, error);
-		return error;
-	}
+	if (error)
+		return dev_err_probe(&pdev->dev, error,
+				     "Failed to request IRQ %d\n", irq);
 
 	error = dev_pm_set_wake_irq(&pdev->dev, irq);
 	if (error)
@@ -251,11 +243,9 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 		device_init_wakeup(&pdev->dev, true);
 
 	error = input_register_device(onkey->input);
-	if (error) {
-		dev_err(&pdev->dev,
-			"Failed to register input device: %d\n", error);
-		return error;
-	}
+	if (error)
+		return dev_err_probe(&pdev->dev, error,
+				     "Failed to register input device\n");
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related

* [PATCH 3/3] Input: da9063 - Add polling support
From: Biju Das @ 2023-12-11 16:57 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Biju Das, Support Opensource, linux-input, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc
In-Reply-To: <20231211165708.161808-1-biju.das.jz@bp.renesas.com>

On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ
populated by default. Add polling support.

While at it, doing some cleanups in da9063_poll_on().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/input/misc/da9063_onkey.c | 88 +++++++++++++++++++++++--------
 1 file changed, 66 insertions(+), 22 deletions(-)

diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
index 536220662b38..b9bb8c1cb758 100644
--- a/drivers/input/misc/da9063_onkey.c
+++ b/drivers/input/misc/da9063_onkey.c
@@ -19,6 +19,8 @@
 #include <linux/mfd/da9062/core.h>
 #include <linux/mfd/da9062/registers.h>
 
+#define DA9062_KEY_THRESHOLD_MSEC	(200)
+
 struct da906x_chip_config {
 	/* REGS */
 	int onkey_status;
@@ -42,6 +44,8 @@ struct da9063_onkey {
 	const struct da906x_chip_config *config;
 	char phys[32];
 	bool key_power;
+	unsigned int poll_interval;
+	unsigned int key_threshold_release_time;
 };
 
 static const struct da906x_chip_config da9063_regs = {
@@ -86,15 +90,27 @@ static void da9063_poll_on(struct work_struct *work)
 	int error;
 
 	/* Poll to see when the pin is released */
-	error = regmap_read(onkey->regmap,
-			    config->onkey_status,
-			    &val);
+	error = regmap_read(onkey->regmap, config->onkey_status, &val);
 	if (error) {
-		dev_err(onkey->dev,
-			"Failed to read ON status: %d\n", error);
+		dev_err(onkey->dev, "Failed to read ON status: %d\n", error);
 		goto err_poll;
 	}
 
+	if (onkey->poll_interval &&
+	    onkey->key_threshold_release_time <= DA9062_KEY_THRESHOLD_MSEC) {
+		/* detect short press or long key press */
+		if (!(val & config->onkey_nonkey_mask)) {
+			input_report_key(onkey->input, KEY_POWER, 0);
+			input_sync(onkey->input);
+			onkey->key_threshold_release_time = 0;
+			dev_dbg(onkey->dev, "KEY_POWER short press.\n");
+		} else {
+			schedule_delayed_work(&onkey->work, msecs_to_jiffies(50));
+			onkey->key_threshold_release_time += 50;
+		}
+		return;
+	}
+
 	if (!(val & config->onkey_nonkey_mask)) {
 		error = regmap_update_bits(onkey->regmap,
 					   config->onkey_pwr_signalling,
@@ -177,6 +193,21 @@ static irqreturn_t da9063_onkey_irq_handler(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void da9063_onkey_polled_poll(struct input_dev *input)
+{
+	struct da9063_onkey *onkey = input_get_drvdata(input);
+	const struct da906x_chip_config *config = onkey->config;
+	unsigned int val;
+	int error;
+
+	error = regmap_read(onkey->regmap, config->onkey_status, &val);
+	if (onkey->key_power && !error && (val & config->onkey_nonkey_mask)) {
+		input_report_key(onkey->input, KEY_POWER, 1);
+		input_sync(onkey->input);
+		schedule_delayed_work(&onkey->work, 0);
+	}
+}
+
 static int da9063_onkey_probe(struct platform_device *pdev)
 {
 	struct da9063_onkey *onkey;
@@ -222,25 +253,38 @@ static int da9063_onkey_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, error,
 				     "Failed to add cancel poll action\n");
 
-	irq = platform_get_irq_byname(pdev, "ONKEY");
-	if (irq < 0)
+	irq = platform_get_irq_byname_optional(pdev, "ONKEY");
+	if (irq != -ENXIO)
 		return irq;
 
-	error = devm_request_threaded_irq(&pdev->dev, irq,
-					  NULL, da9063_onkey_irq_handler,
-					  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-					  "ONKEY", onkey);
-	if (error)
-		return dev_err_probe(&pdev->dev, error,
-				     "Failed to request IRQ %d\n", irq);
-
-	error = dev_pm_set_wake_irq(&pdev->dev, irq);
-	if (error)
-		dev_warn(&pdev->dev,
-			 "Failed to set IRQ %d as a wake IRQ: %d\n",
-			 irq, error);
-	else
-		device_init_wakeup(&pdev->dev, true);
+	if (irq >= 0) {
+		error = devm_request_threaded_irq(&pdev->dev, irq,
+						  NULL, da9063_onkey_irq_handler,
+						  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						  "ONKEY", onkey);
+		if (error)
+			return dev_err_probe(&pdev->dev, error,
+					     "Failed to request IRQ %d\n", irq);
+
+		error = dev_pm_set_wake_irq(&pdev->dev, irq);
+		if (error)
+			dev_warn(&pdev->dev,
+				 "Failed to set IRQ %d as a wake IRQ: %d\n",
+				 irq, error);
+		else
+			device_init_wakeup(&pdev->dev, true);
+	} else {
+		input_set_drvdata(onkey->input, onkey);
+		device_property_read_u32(&pdev->dev, "poll-interval",
+					 &onkey->poll_interval);
+		error = input_setup_polling(onkey->input,
+					    da9063_onkey_polled_poll);
+		if (error)
+			return dev_err_probe(&pdev->dev, error,
+					     "unable to set up polling\n");
+
+		input_set_poll_interval(onkey->input, onkey->poll_interval);
+	}
 
 	error = input_register_device(onkey->input);
 	if (error)
-- 
2.25.1


^ permalink raw reply related

* RE: [PATCH 3/3] Input: da9063 - Add polling support
From: Biju Das @ 2023-12-11 17:07 UTC (permalink / raw)
  To: Biju Das, Dmitry Torokhov
  Cc: Support Opensource, linux-input@vger.kernel.org,
	Geert Uytterhoeven, Prabhakar Mahadev Lad, biju.das.au,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <20231211165708.161808-4-biju.das.jz@bp.renesas.com>

Hi All,

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Monday, December 11, 2023 4:57 PM
> Subject: [PATCH 3/3] Input: da9063 - Add polling support
> 
> On some platforms (eg: RZ/{G2UL,Five} SMARC EVK), there is no IRQ
> populated by default. Add polling support.
> 
> While at it, doing some cleanups in da9063_poll_on().
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/input/misc/da9063_onkey.c | 88 +++++++++++++++++++++++--------
>  1 file changed, 66 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/input/misc/da9063_onkey.c
> b/drivers/input/misc/da9063_onkey.c
> index 536220662b38..b9bb8c1cb758 100644
> --- a/drivers/input/misc/da9063_onkey.c
> +++ b/drivers/input/misc/da9063_onkey.c
> @@ -19,6 +19,8 @@
>  #include <linux/mfd/da9062/core.h>
>  #include <linux/mfd/da9062/registers.h>
> 
> +#define DA9062_KEY_THRESHOLD_MSEC	(200)
> +
>  struct da906x_chip_config {
>  	/* REGS */
>  	int onkey_status;
> @@ -42,6 +44,8 @@ struct da9063_onkey {
>  	const struct da906x_chip_config *config;
>  	char phys[32];
>  	bool key_power;
> +	unsigned int poll_interval;
> +	unsigned int key_threshold_release_time;
>  };
> 
>  static const struct da906x_chip_config da9063_regs = { @@ -86,15 +90,27
> @@ static void da9063_poll_on(struct work_struct *work)
>  	int error;
> 
>  	/* Poll to see when the pin is released */
> -	error = regmap_read(onkey->regmap,
> -			    config->onkey_status,
> -			    &val);
> +	error = regmap_read(onkey->regmap, config->onkey_status, &val);
>  	if (error) {
> -		dev_err(onkey->dev,
> -			"Failed to read ON status: %d\n", error);
> +		dev_err(onkey->dev, "Failed to read ON status: %d\n", error);
>  		goto err_poll;
>  	}
> 
> +	if (onkey->poll_interval &&
> +	    onkey->key_threshold_release_time <= DA9062_KEY_THRESHOLD_MSEC)
> {
> +		/* detect short press or long key press */
> +		if (!(val & config->onkey_nonkey_mask)) {
> +			input_report_key(onkey->input, KEY_POWER, 0);
> +			input_sync(onkey->input);
> +			onkey->key_threshold_release_time = 0;
> +			dev_dbg(onkey->dev, "KEY_POWER short press.\n");
> +		} else {
> +			schedule_delayed_work(&onkey->work,
> msecs_to_jiffies(50));
> +			onkey->key_threshold_release_time += 50;
> +		}
> +		return;
> +	}
> +
>  	if (!(val & config->onkey_nonkey_mask)) {
>  		error = regmap_update_bits(onkey->regmap,
>  					   config->onkey_pwr_signalling,
> @@ -177,6 +193,21 @@ static irqreturn_t da9063_onkey_irq_handler(int irq,
> void *data)
>  	return IRQ_HANDLED;
>  }
> 
> +static void da9063_onkey_polled_poll(struct input_dev *input) {
> +	struct da9063_onkey *onkey = input_get_drvdata(input);
> +	const struct da906x_chip_config *config = onkey->config;
> +	unsigned int val;
> +	int error;
> +
> +	error = regmap_read(onkey->regmap, config->onkey_status, &val);
> +	if (onkey->key_power && !error && (val & config->onkey_nonkey_mask))
> {
> +		input_report_key(onkey->input, KEY_POWER, 1);
> +		input_sync(onkey->input);
> +		schedule_delayed_work(&onkey->work, 0);
> +	}
> +}
> +
>  static int da9063_onkey_probe(struct platform_device *pdev)  {
>  	struct da9063_onkey *onkey;
> @@ -222,25 +253,38 @@ static int da9063_onkey_probe(struct platform_device
> *pdev)
>  		return dev_err_probe(&pdev->dev, error,
>  				     "Failed to add cancel poll action\n");
> 
> -	irq = platform_get_irq_byname(pdev, "ONKEY");
> -	if (irq < 0)
> +	irq = platform_get_irq_byname_optional(pdev, "ONKEY");
> +	if (irq != -ENXIO)
>  		return irq;

Oops, this check is wrong for IRQ case.

I will send v2, once I get feedback for this patch series.
It should be like [1]

[1]
 https://patchwork.kernel.org/project/linux-renesas-soc/patch/20231204130504.126787-2-biju.das.jz@bp.renesas.com/

Cheers,
Biju

> 
> -	error = devm_request_threaded_irq(&pdev->dev, irq,
> -					  NULL, da9063_onkey_irq_handler,
> -					  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -					  "ONKEY", onkey);
> -	if (error)
> -		return dev_err_probe(&pdev->dev, error,
> -				     "Failed to request IRQ %d\n", irq);
> -
> -	error = dev_pm_set_wake_irq(&pdev->dev, irq);
> -	if (error)
> -		dev_warn(&pdev->dev,
> -			 "Failed to set IRQ %d as a wake IRQ: %d\n",
> -			 irq, error);
> -	else
> -		device_init_wakeup(&pdev->dev, true);
> +	if (irq >= 0) {
> +		error = devm_request_threaded_irq(&pdev->dev, irq,
> +						  NULL, da9063_onkey_irq_handler,
> +						  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +						  "ONKEY", onkey);
> +		if (error)
> +			return dev_err_probe(&pdev->dev, error,
> +					     "Failed to request IRQ %d\n", irq);
> +
> +		error = dev_pm_set_wake_irq(&pdev->dev, irq);
> +		if (error)
> +			dev_warn(&pdev->dev,
> +				 "Failed to set IRQ %d as a wake IRQ: %d\n",
> +				 irq, error);
> +		else
> +			device_init_wakeup(&pdev->dev, true);
> +	} else {
> +		input_set_drvdata(onkey->input, onkey);
> +		device_property_read_u32(&pdev->dev, "poll-interval",
> +					 &onkey->poll_interval);
> +		error = input_setup_polling(onkey->input,
> +					    da9063_onkey_polled_poll);
> +		if (error)
> +			return dev_err_probe(&pdev->dev, error,
> +					     "unable to set up polling\n");
> +
> +		input_set_poll_interval(onkey->input, onkey->poll_interval);
> +	}
> 
>  	error = input_register_device(onkey->input);
>  	if (error)
> --
> 2.25.1


^ permalink raw reply

* Re: [PATCH v5 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
From: Conor Dooley @ 2023-12-11 18:37 UTC (permalink / raw)
  To: Biju Das
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, 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: <20231210134717.94020-9-biju.das.jz@bp.renesas.com>

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

On Sun, Dec 10, 2023 at 01:47:17PM +0000, Biju Das wrote:
> Convert the da9062 PMIC device tree binding documentation to json-schema.
> 
> Document the missing gpio child node for da9062.
> 
> While at it, update description with link to product information and
> example.
> 
> The missing child node with of_compatible defined in MFD_CELL_OF is
> causing the below warning message:
> da9062-gpio: Failed to locate of_node [id: -1]
> 
> So, make all child nodes with of_compatible defined in struct mfd_cell
> as required property for da906{1,2} devices.

> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio:
> +    type: object
> +    additionalProperties: false
> +    properties:
> +      compatible:
> +        const: dlg,da9062-gpio

> +  - |
> +    #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>;
> +        gpio-controller;
> +        #gpio-cells = <2>;

> +        gpio {
> +          compatible = "dlg,da9062-gpio";
> +        };

I know you had some conversation with Krzysztof, but I still don;t
really follow this. Why is the parent, rather than the child, the one
that gets the "gpio-controller" and "#gpio-cells" properties? The commit
message just mentions why missing child node was added, but not the
reason for the gpio properties being added at what appears to be the
"wrong" level.

Cheers,
Conor.


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

^ permalink raw reply

* RE: [PATCH v5 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
From: Biju Das @ 2023-12-11 18:51 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Steve Twiss, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	Geert Uytterhoeven, Prabhakar Mahadev Lad, biju.das.au,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <20231211-dissuade-skirt-5961ef525497@spud>

Hi Conor Dooley,

Thanks for the feedback.

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Monday, December 11, 2023 6:38 PM
> Subject: Re: [PATCH v5 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062
> to json-schema
> 
> On Sun, Dec 10, 2023 at 01:47:17PM +0000, Biju Das wrote:
> > Convert the da9062 PMIC device tree binding documentation to json-
> schema.
> >
> > Document the missing gpio child node for da9062.
> >
> > While at it, update description with link to product information and
> > example.
> >
> > The missing child node with of_compatible defined in MFD_CELL_OF is
> > causing the below warning message:
> > da9062-gpio: Failed to locate of_node [id: -1]
> >
> > So, make all child nodes with of_compatible defined in struct mfd_cell
> > as required property for da906{1,2} devices.
> 
> > +  gpio-controller: true
> > +
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  gpio:
> > +    type: object
> > +    additionalProperties: false
> > +    properties:
> > +      compatible:
> > +        const: dlg,da9062-gpio
> 
> > +  - |
> > +    #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>;
> > +        gpio-controller;
> > +        #gpio-cells = <2>;
> 
> > +        gpio {
> > +          compatible = "dlg,da9062-gpio";
> > +        };
> 
> I know you had some conversation with Krzysztof, but I still don;t really
> follow this. Why is the parent, rather than the child, the one that gets
> the "gpio-controller" and "#gpio-cells" properties? The commit message
> just mentions why missing child node was added, but not the reason for the
> gpio properties being added at what appears to be the "wrong" level.


Please see [1], The driver is checking against parent "gpio-controller"

[1] https://elixir.bootlin.com/linux/v6.0-rc4/source/drivers/pinctrl/pinctrl-da9062.c#L270


Cheers,
Biju

^ permalink raw reply

* Re: [PATCH 2/3] Input: da9063 - Use dev_err_probe()
From: Sergey Shtylyov @ 2023-12-11 19:05 UTC (permalink / raw)
  To: Biju Das, Dmitry Torokhov
  Cc: Support Opensource, linux-input, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc
In-Reply-To: <20231211165708.161808-3-biju.das.jz@bp.renesas.com>

On 12/11/23 7:57 PM, Biju Das wrote:

> Replace dev_err()->dev_err_probe() to simpilfy probe().

  Simplify. :-)

> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
[...]

MBR, Sergey

^ permalink raw reply

* RE: [PATCH 2/3] Input: da9063 - Use dev_err_probe()
From: Biju Das @ 2023-12-11 19:07 UTC (permalink / raw)
  To: Sergey Shtylyov, Dmitry Torokhov
  Cc: Support Opensource, linux-input@vger.kernel.org,
	Geert Uytterhoeven, Prabhakar Mahadev Lad, biju.das.au,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <71cb51b3-7145-cf03-9291-5ff8acdaa05e@omp.ru>

Hi Sergey Shtylyov,

Thanks for the feedback.

> -----Original Message-----
> From: Sergey Shtylyov <s.shtylyov@omp.ru>
> Sent: Monday, December 11, 2023 7:05 PM
> Subject: Re: [PATCH 2/3] Input: da9063 - Use dev_err_probe()
> 
> On 12/11/23 7:57 PM, Biju Das wrote:
> 
> > Replace dev_err()->dev_err_probe() to simpilfy probe().
> 
>   Simplify. :-)

OK will fix it in v2.

Cheers,
Biju
> 
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> [...]
> 
> MBR, Sergey

^ permalink raw reply

* RE: [PATCH v5 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
From: Biju Das @ 2023-12-11 19:10 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Steve Twiss, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	Geert Uytterhoeven, Prabhakar Mahadev Lad, biju.das.au,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <20231211-dissuade-skirt-5961ef525497@spud>


Hi Conor Dooley,

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Monday, December 11, 2023 6:38 PM
> Subject: Re: [PATCH v5 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062
> to json-schema
> 
> On Sun, Dec 10, 2023 at 01:47:17PM +0000, Biju Das wrote:
> > Convert the da9062 PMIC device tree binding documentation to json-
> schema.
> >
> > Document the missing gpio child node for da9062.
> >
> > While at it, update description with link to product information and
> > example.
> >
> > The missing child node with of_compatible defined in MFD_CELL_OF is
> > causing the below warning message:
> > da9062-gpio: Failed to locate of_node [id: -1]
> >
> > So, make all child nodes with of_compatible defined in struct mfd_cell
> > as required property for da906{1,2} devices.
> 
> > +  gpio-controller: true
> > +
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  gpio:
> > +    type: object
> > +    additionalProperties: false
> > +    properties:
> > +      compatible:
> > +        const: dlg,da9062-gpio
> 
> > +  - |
> > +    #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>;
> > +        gpio-controller;
> > +        #gpio-cells = <2>;
> 
> > +        gpio {
> > +          compatible = "dlg,da9062-gpio";
> > +        };
> 
> I know you had some conversation with Krzysztof, but I still don;t really
> follow this. Why is the parent, rather than the child, the one that gets
> the "gpio-controller" and "#gpio-cells" properties? The commit message
> just mentions why missing child node was added, but not the reason for the
> gpio properties being added at what appears to be the "wrong" level.

The original binding has this in parent. See [1] and driver is based on this documentation[2]

[1]

https://elixir.bootlin.com/linux/v6.0-rc4/source/Documentation/devicetree/bindings/mfd/da9062.txt#L44

[2]

https://elixir.bootlin.com/linux/v6.0-rc4/source/drivers/pinctrl/pinctrl-da9062.c#L270


Cheers,
Biju

^ permalink raw reply

* [PATCH v2] Input: omap4-keypad: react on keypresses if device is runtime-suspended
From: Andreas Kemnade @ 2023-12-11 22:17 UTC (permalink / raw)
  To: dmitry.torokhov, Jonathan.Cameron, tony, robh, andreas, frank.li,
	u.kleine-koenig, linux-input, linux-kernel

According to SWPU235AB, table 26-6, fclk is required to generate events
at least on OMAP4460, so keep fclk enabled all the time the device
is opened.

Suggested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Reviewed-by: Tony Lindgren <tony@atomide.com>
---
Changes in v2:
- error check for clk_prepare_enable() although it should never
  be reached without that clock being enabled, but lets be
  careful.
Changes since RFC:
- add R-by:
 drivers/input/keyboard/omap4-keypad.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index d3f8688fdd9c3..040b340995d89 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/clk.h>
 #include <linux/errno.h>
 #include <linux/io.h>
 #include <linux/of.h>
@@ -83,6 +84,7 @@ struct omap4_keypad {
 	bool no_autorepeat;
 	u64 keys;
 	unsigned short *keymap;
+	struct clk *fck;
 };
 
 static int kbd_readl(struct omap4_keypad *keypad_data, u32 offset)
@@ -209,6 +211,10 @@ static int omap4_keypad_open(struct input_dev *input)
 	if (error)
 		return error;
 
+	error = clk_prepare_enable(keypad_data->fck);
+	if (error)
+		goto out;
+
 	disable_irq(keypad_data->irq);
 
 	kbd_writel(keypad_data, OMAP4_KBD_CTRL,
@@ -226,10 +232,11 @@ static int omap4_keypad_open(struct input_dev *input)
 
 	enable_irq(keypad_data->irq);
 
+out:
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 
-	return 0;
+	return error;
 }
 
 static void omap4_keypad_stop(struct omap4_keypad *keypad_data)
@@ -258,6 +265,7 @@ static void omap4_keypad_close(struct input_dev *input)
 	disable_irq(keypad_data->irq);
 	omap4_keypad_stop(keypad_data);
 	enable_irq(keypad_data->irq);
+	clk_disable_unprepare(keypad_data->fck);
 
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
@@ -356,6 +364,11 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	}
 
 	keypad_data->irq = irq;
+	keypad_data->fck = devm_clk_get(&pdev->dev, "fck");
+	if (IS_ERR(keypad_data->fck))
+		return dev_err_probe(&pdev->dev, PTR_ERR(keypad_data->fck),
+				     "unable to get fck");
+
 	mutex_init(&keypad_data->lock);
 	platform_set_drvdata(pdev, keypad_data);
 
-- 
2.39.2


^ permalink raw reply related

* [PATCH v7 00/12] Introduce PMF Smart PC Solution Builder Feature
From: Shyam Sundar S K @ 2023-12-12  1:46 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K

Smart PC Solutions Builder allows for OEM to define a large number of
custom system states to dynamically switch to. The system states are
referred to as policies, and multiple policies can be loaded onto the
system at any given time, however only one policy can be active at a
given time.

Policy is a combination of PMF input and output capabilities. The inputs
are the incoming information from the other kernel subsystems like LID
state, Sensor info, GPU info etc and the actions are the updating the 
power limits of SMU etc.

The policy binary is signed and encrypted by a special key from AMD. This
policy binary shall have the inputs and outputs which the OEMs can build
for the platform customization that can enhance the user experience and
system behavior.

This series adds the initial support for Smart PC solution to PMF driver.

Note that, on platforms where CnQF and Smart PC is advertised, Smart PC
shall have higher precedence and same applies for Auto Mode.

v6->v7:
---------
 - handle buffer free during suspend/resume
 - Move Smart PC checks within Smart PC init function
 - realloc a updated buffer size during the side load.
 - Drop patches from 13/15 to 15/15 of V6 series

v5->v6:
---------
 - Add Ilpo's and Mario's Reviewed-by tags
 - Drop 13/17 and 14/17 patches from this series which are GPU centric
 - Drop separate checks for battery handling.
 - Handle SFH failure cases

v4->v5:
---------
 - Remove PMF-GPU interface from amdgpu driver and add DRM/backlight
   changes within PMF
 - Add module_softdep for AMDGPU
 - remove error checks for debugfs_create_file()
 - Add "Reviewed-by:" tags
 - Add kerneldoc for kernel-wide headers
 - Add checks for acpi_backlight_native
 - Add early return for SFH path
 - other cosmetic changes
 
v3->v4:
---------
- Split v3 9/16 into 2 patches, that addresses using generic fn names
- Add softdep [Ilpo] instead of request_module()
- return proper ACPI status [Mario]
- Update comments in code [Mario]
- Remove missed double _ remarks
- handle battery status branches [Ilpo]
- Address KASAN problems 

v2->v3:
---------
- Remove pci_get_device() for getting gpu handle
- add .suspend handler for pmf driver
- remove unwanted type caste
- Align comments, spaces etc.
- add wrapper for print_hex_dump_debug()
- Remove lkp tags in commit-msg
- Add macros for magic numbers
- use right format specifiers for printing
- propagate error codes back to the caller
- remove unwanted comments

v1->v2:
---------
- Remove __func__ macros
- Remove manual function names inside prints
- Handle tee_shm_get_va() failure
- Remove double _
- Add meaningful prints
- pass amd_pmf_set_dram_addr() failure errors
- Add more information to commit messages
- use right format specifiers
- use devm_ioremap() instead of ioremap()
- address unsigned long vs u32 problems
- Fix lkp reported issues
- Add amd_pmf_remove_pb() to remove the debugfs files created(if any).
- Make amd_pmf_open_pb() as static.
- Add cooling device APIs for controlling amdgpu backlight
- handle amd_pmf_apply_policies() failures
- Split v1 14/15 into 2 patches further
- use linux/units.h for better handling
- add "depends on" AMD_SFH_HID for interaction with SFH
- other cosmetic remarks

Shyam Sundar S K (12):
  platform/x86/amd/pmf: Add PMF TEE interface
  platform/x86/amd/pmf: Add support for PMF-TA interaction
  platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
  platform/x86/amd/pmf: Add support for PMF Policy Binary
  platform/x86/amd/pmf: change amd_pmf_init_features() call sequence
  platform/x86/amd/pmf: Add support to get inputs from other subsystems
  platform/x86/amd/pmf: Add support update p3t limit
  platform/x86/amd/pmf: Add support to update system state
  platform/x86/amd/pmf: Make source_as_str() as non-static
  platform/x86/amd/pmf: Add facility to dump TA inputs
  platform/x86/amd/pmf: Add capability to sideload of policy binary
  platform/x86/amd/pmf: dump policy binary data

 Documentation/admin-guide/index.rst   |   1 +
 Documentation/admin-guide/pmf.rst     |  24 ++
 drivers/platform/x86/amd/pmf/Kconfig  |   1 +
 drivers/platform/x86/amd/pmf/Makefile |   3 +-
 drivers/platform/x86/amd/pmf/acpi.c   |  37 ++
 drivers/platform/x86/amd/pmf/core.c   |  52 ++-
 drivers/platform/x86/amd/pmf/pmf.h    | 203 +++++++++++
 drivers/platform/x86/amd/pmf/spc.c    | 158 +++++++++
 drivers/platform/x86/amd/pmf/sps.c    |   5 +-
 drivers/platform/x86/amd/pmf/tee-if.c | 469 ++++++++++++++++++++++++++
 10 files changed, 936 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/admin-guide/pmf.rst
 create mode 100644 drivers/platform/x86/amd/pmf/spc.c
 create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c

-- 
2.25.1


^ permalink raw reply

* [PATCH v7 01/12] platform/x86/amd/pmf: Add PMF TEE interface
From: Shyam Sundar S K @ 2023-12-12  1:46 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K
In-Reply-To: <20231212014705.2017474-1-Shyam-sundar.S-k@amd.com>

AMD PMF driver loads the PMF TA (Trusted Application) into the AMD
ASP's (AMD Security Processor) TEE (Trusted Execution Environment).

PMF Trusted Application is a secured firmware placed under
/lib/firmware/amdtee gets loaded only when the TEE environment is
initialized. Add the initial code path to build these pipes.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/Kconfig  |   1 +
 drivers/platform/x86/amd/pmf/Makefile |   3 +-
 drivers/platform/x86/amd/pmf/core.c   |  10 ++-
 drivers/platform/x86/amd/pmf/pmf.h    |  16 ++++
 drivers/platform/x86/amd/pmf/tee-if.c | 105 ++++++++++++++++++++++++++
 5 files changed, 130 insertions(+), 5 deletions(-)
 create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c

diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index 3064bc8ea167..32a029e8db80 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -9,6 +9,7 @@ config AMD_PMF
 	depends on POWER_SUPPLY
 	depends on AMD_NB
 	select ACPI_PLATFORM_PROFILE
+	depends on TEE
 	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/Makefile b/drivers/platform/x86/amd/pmf/Makefile
index fdededf54392..d2746ee7369f 100644
--- a/drivers/platform/x86/amd/pmf/Makefile
+++ b/drivers/platform/x86/amd/pmf/Makefile
@@ -6,4 +6,5 @@
 
 obj-$(CONFIG_AMD_PMF) += amd-pmf.o
 amd-pmf-objs := core.o acpi.o sps.o \
-		auto-mode.o cnqf.o
+		auto-mode.o cnqf.o \
+		tee-if.o
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 78ed3ee22555..ec92d1cc0dac 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -309,13 +309,13 @@ static void amd_pmf_init_features(struct amd_pmf_dev *dev)
 		dev_dbg(dev->dev, "SPS enabled and Platform Profiles registered\n");
 	}
 
-	/* Enable Auto Mode */
-	if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
+	if (!amd_pmf_init_smart_pc(dev)) {
+		dev_dbg(dev->dev, "Smart PC Solution Enabled\n");
+	} else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
 		amd_pmf_init_auto_mode(dev);
 		dev_dbg(dev->dev, "Auto Mode Init done\n");
 	} else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
 			  is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_DC)) {
-		/* Enable Cool n Quiet Framework (CnQF) */
 		ret = amd_pmf_init_cnqf(dev);
 		if (ret)
 			dev_warn(dev->dev, "CnQF Init failed\n");
@@ -330,7 +330,9 @@ static void amd_pmf_deinit_features(struct amd_pmf_dev *dev)
 		amd_pmf_deinit_sps(dev);
 	}
 
-	if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
+	if (!dev->smart_pc_enabled) {
+		amd_pmf_deinit_smart_pc(dev);
+	} else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
 		amd_pmf_deinit_auto_mode(dev);
 	} else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
 			  is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_DC)) {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index deba88e6e4c8..bd40458937ba 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -179,6 +179,12 @@ struct amd_pmf_dev {
 	bool cnqf_enabled;
 	bool cnqf_supported;
 	struct notifier_block pwr_src_notifier;
+	/* Smart PC solution builder */
+	struct tee_context *tee_ctx;
+	struct tee_shm *fw_shm_pool;
+	u32 session_id;
+	void *shbuf;
+	bool smart_pc_enabled;
 };
 
 struct apmf_sps_prop_granular {
@@ -389,6 +395,13 @@ struct apmf_dyn_slider_output {
 	struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
 } __packed;
 
+struct ta_pmf_shared_memory {
+	int command_id;
+	int resp_id;
+	u32 pmf_result;
+	u32 if_version;
+};
+
 /* Core Layer */
 int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
 void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev);
@@ -433,4 +446,7 @@ void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev);
 int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t time_lapsed_ms);
 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);
 #endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
new file mode 100644
index 000000000000..6ec8c3726624
--- /dev/null
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Platform Management Framework Driver - TEE Interface
+ *
+ * Copyright (c) 2023, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ */
+
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+#include "pmf.h"
+
+#define MAX_TEE_PARAM	4
+static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
+						0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
+
+static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+	return ver->impl_id == TEE_IMPL_ID_AMDTEE;
+}
+
+static int amd_pmf_ta_open_session(struct tee_context *ctx, u32 *id)
+{
+	struct tee_ioctl_open_session_arg sess_arg = {};
+	int rc;
+
+	export_uuid(sess_arg.uuid, &amd_pmf_ta_uuid);
+	sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
+	sess_arg.num_params = 0;
+
+	rc = tee_client_open_session(ctx, &sess_arg, NULL);
+	if (rc < 0 || sess_arg.ret != 0) {
+		pr_err("Failed to open TEE session err:%#x, rc:%d\n", sess_arg.ret, rc);
+		return rc;
+	}
+
+	*id = sess_arg.session;
+
+	return rc;
+}
+
+static int amd_pmf_tee_init(struct amd_pmf_dev *dev)
+{
+	u32 size;
+	int ret;
+
+	dev->tee_ctx = tee_client_open_context(NULL, amd_pmf_amdtee_ta_match, NULL, NULL);
+	if (IS_ERR(dev->tee_ctx)) {
+		dev_err(dev->dev, "Failed to open TEE context\n");
+		return PTR_ERR(dev->tee_ctx);
+	}
+
+	ret = amd_pmf_ta_open_session(dev->tee_ctx, &dev->session_id);
+	if (ret) {
+		dev_err(dev->dev, "Failed to open TA session (%d)\n", ret);
+		ret = -EINVAL;
+		goto out_ctx;
+	}
+
+	size = sizeof(struct ta_pmf_shared_memory);
+	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");
+		ret = PTR_ERR(dev->fw_shm_pool);
+		goto out_sess;
+	}
+
+	dev->shbuf = tee_shm_get_va(dev->fw_shm_pool, 0);
+	if (IS_ERR(dev->shbuf)) {
+		dev_err(dev->dev, "Failed to get TEE virtual address\n");
+		ret = PTR_ERR(dev->shbuf);
+		goto out_shm;
+	}
+	dev_dbg(dev->dev, "TEE init done\n");
+
+	return 0;
+
+out_shm:
+	tee_shm_free(dev->fw_shm_pool);
+out_sess:
+	tee_client_close_session(dev->tee_ctx, dev->session_id);
+out_ctx:
+	tee_client_close_context(dev->tee_ctx);
+
+	return ret;
+}
+
+static void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
+{
+	tee_shm_free(dev->fw_shm_pool);
+	tee_client_close_session(dev->tee_ctx, dev->session_id);
+	tee_client_close_context(dev->tee_ctx);
+}
+
+int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
+{
+	return amd_pmf_tee_init(dev);
+}
+
+void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
+{
+	amd_pmf_tee_deinit(dev);
+}
-- 
2.25.1


^ permalink raw reply related

* [PATCH v7 02/12] platform/x86/amd/pmf: Add support for PMF-TA interaction
From: Shyam Sundar S K @ 2023-12-12  1:46 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K
In-Reply-To: <20231212014705.2017474-1-Shyam-sundar.S-k@amd.com>

PMF TA (Trusted Application) loads via the TEE environment into the
AMD ASP.

PMF-TA supports two commands:
1) Init: Initialize the TA with the PMF Smart PC policy binary and
start the policy engine. A policy is a combination of inputs and
outputs, where;
 - the inputs are the changing dynamics of the system like the user
   behaviour, system heuristics etc.
 - the outputs, which are the actions to be set on the system which
   lead to better power management and enhanced user experience.

PMF driver acts as a central manager in this case to supply the
inputs required to the TA (either by getting the information from
the other kernel subsystems or from userland)

2) Enact: Enact the output actions from the TA. The action could be
applying a new thermal limit to boost/throttle the power limits or
change system behavior.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/pmf.h    | 10 +++
 drivers/platform/x86/amd/pmf/tee-if.c | 97 ++++++++++++++++++++++++++-
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index bd40458937ba..a24e34e42032 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -59,6 +59,9 @@
 #define ARG_NONE 0
 #define AVG_SAMPLE_SIZE 3
 
+/* TA macros */
+#define PMF_TA_IF_VERSION_MAJOR				1
+
 /* AMD PMF BIOS interfaces */
 struct apmf_verify_interface {
 	u16 size;
@@ -184,6 +187,7 @@ struct amd_pmf_dev {
 	struct tee_shm *fw_shm_pool;
 	u32 session_id;
 	void *shbuf;
+	struct delayed_work pb_work;
 	bool smart_pc_enabled;
 };
 
@@ -395,6 +399,12 @@ struct apmf_dyn_slider_output {
 	struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
 } __packed;
 
+/* Command ids for TA communication */
+enum ta_pmf_command {
+	TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE,
+	TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES,
+};
+
 struct ta_pmf_shared_memory {
 	int command_id;
 	int resp_id;
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 6ec8c3726624..4036f435f1e2 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -13,9 +13,96 @@
 #include "pmf.h"
 
 #define MAX_TEE_PARAM	4
+
+/* Policy binary actions sampling frequency (in ms) */
+static int pb_actions_ms = MSEC_PER_SEC;
+#ifdef CONFIG_AMD_PMF_DEBUG
+module_param(pb_actions_ms, int, 0644);
+MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)");
+#endif
+
 static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
 						0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
 
+static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
+				 struct tee_ioctl_invoke_arg *arg,
+				 struct tee_param *param)
+{
+	memset(arg, 0, sizeof(*arg));
+	memset(param, 0, MAX_TEE_PARAM * sizeof(*param));
+
+	arg->func = cmd;
+	arg->session = dev->session_id;
+	arg->num_params = MAX_TEE_PARAM;
+
+	/* Fill invoke cmd params */
+	param[0].u.memref.size = sizeof(struct ta_pmf_shared_memory);
+	param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
+	param[0].u.memref.shm = dev->fw_shm_pool;
+	param[0].u.memref.shm_offs = 0;
+}
+
+static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
+{
+	struct ta_pmf_shared_memory *ta_sm = NULL;
+	struct tee_param param[MAX_TEE_PARAM];
+	struct tee_ioctl_invoke_arg arg;
+	int ret = 0;
+
+	if (!dev->tee_ctx)
+		return -ENODEV;
+
+	ta_sm = dev->shbuf;
+	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_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES, &arg, param);
+
+	ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
+	if (ret < 0 || arg.ret != 0) {
+		dev_err(dev->dev, "TEE enact cmd failed. err: %x, ret:%d\n", arg.ret, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+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 tee_ioctl_invoke_arg arg;
+	int ret = 0;
+
+	if (!dev->tee_ctx) {
+		dev_err(dev->dev, "Failed to get TEE context\n");
+		return -ENODEV;
+	}
+
+	ta_sm = dev->shbuf;
+	ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE;
+	ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
+
+	amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE, &arg, param);
+
+	ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
+	if (ret < 0 || arg.ret != 0) {
+		dev_err(dev->dev, "Failed to invoke TEE init cmd. err: %x, ret:%d\n", arg.ret, ret);
+		return ret;
+	}
+
+	return ta_sm->pmf_result;
+}
+
+static void amd_pmf_invoke_cmd(struct work_struct *work)
+{
+	struct amd_pmf_dev *dev = container_of(work, struct amd_pmf_dev, pb_work.work);
+
+	amd_pmf_invoke_cmd_enact(dev);
+	schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms));
+}
+
 static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
 {
 	return ver->impl_id == TEE_IMPL_ID_AMDTEE;
@@ -96,10 +183,18 @@ static void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
 
 int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
 {
-	return amd_pmf_tee_init(dev);
+	int ret;
+
+	ret = amd_pmf_tee_init(dev);
+	if (ret)
+		return ret;
+
+	INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
+	return 0;
 }
 
 void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
 {
+	cancel_delayed_work_sync(&dev->pb_work);
 	amd_pmf_tee_deinit(dev);
 }
-- 
2.25.1


^ permalink raw reply related

* [PATCH v7 03/12] platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
From: Shyam Sundar S K @ 2023-12-12  1:46 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K
In-Reply-To: <20231212014705.2017474-1-Shyam-sundar.S-k@amd.com>

In the current code, the metrics table information was required only
for auto-mode or CnQF at a given time. Hence keeping the return type
of amd_pmf_set_dram_addr() as static made sense.

But with the addition of Smart PC builder feature, the metrics table
information has to be shared by the Smart PC also and this feature
resides outside of core.c.

To make amd_pmf_set_dram_addr() visible outside of core.c make it
as a non-static function and move the allocation of memory for
metrics table from amd_pmf_init_metrics_table() to amd_pmf_set_dram_addr()
as amd_pmf_set_dram_addr() is the common function to set the DRAM
address.

Add a suspend handler that can free up the allocated memory for getting
the metrics table information.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/core.c | 39 ++++++++++++++++++++++-------
 drivers/platform/x86/amd/pmf/pmf.h  |  1 +
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index ec92d1cc0dac..9953619a830b 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -251,29 +251,37 @@ static const struct pci_device_id pmf_pci_ids[] = {
 	{ }
 };
 
-static void amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
+int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev, bool alloc_buffer)
 {
 	u64 phys_addr;
 	u32 hi, low;
 
+	/* Get Metrics Table Address */
+	if (alloc_buffer) {
+		dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
+		if (!dev->buf)
+			return -ENOMEM;
+	}
+
 	phys_addr = virt_to_phys(dev->buf);
 	hi = phys_addr >> 32;
 	low = phys_addr & GENMASK(31, 0);
 
 	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
 	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
+
+	return 0;
 }
 
 int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
 {
-	/* Get Metrics Table Address */
-	dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
-	if (!dev->buf)
-		return -ENOMEM;
+	int ret;
 
 	INIT_DELAYED_WORK(&dev->work_buffer, amd_pmf_get_metrics);
 
-	amd_pmf_set_dram_addr(dev);
+	ret = amd_pmf_set_dram_addr(dev, true);
+	if (ret)
+		return ret;
 
 	/*
 	 * Start collecting the metrics data after a small delay
@@ -284,17 +292,30 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
 	return 0;
 }
 
+static int amd_pmf_suspend_handler(struct device *dev)
+{
+	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
+
+	kfree(pdev->buf);
+
+	return 0;
+}
+
 static int amd_pmf_resume_handler(struct device *dev)
 {
 	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
+	int ret;
 
-	if (pdev->buf)
-		amd_pmf_set_dram_addr(pdev);
+	if (pdev->buf) {
+		ret = amd_pmf_set_dram_addr(pdev, false);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }
 
-static DEFINE_SIMPLE_DEV_PM_OPS(amd_pmf_pm, NULL, amd_pmf_resume_handler);
+static DEFINE_SIMPLE_DEV_PM_OPS(amd_pmf_pm, amd_pmf_suspend_handler, amd_pmf_resume_handler);
 
 static void amd_pmf_init_features(struct amd_pmf_dev *dev)
 {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index a24e34e42032..6c1aba5d2027 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -421,6 +421,7 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev);
 int amd_pmf_get_power_source(void);
 int apmf_install_handler(struct amd_pmf_dev *pmf_dev);
 int apmf_os_power_slider_update(struct amd_pmf_dev *dev, u8 flag);
+int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev, bool alloc_buffer);
 
 /* SPS Layer */
 int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v7 04/12] platform/x86/amd/pmf: Add support for PMF Policy Binary
From: Shyam Sundar S K @ 2023-12-12  1:46 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K
In-Reply-To: <20231212014705.2017474-1-Shyam-sundar.S-k@amd.com>

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.

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>
---
 drivers/platform/x86/amd/pmf/Kconfig  |   2 +-
 drivers/platform/x86/amd/pmf/acpi.c   |  37 ++++++
 drivers/platform/x86/amd/pmf/core.c   |   1 +
 drivers/platform/x86/amd/pmf/pmf.h    | 141 +++++++++++++++++++++++
 drivers/platform/x86/amd/pmf/tee-if.c | 158 +++++++++++++++++++++++++-
 5 files changed, 336 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 9953619a830b..c10d40b33667 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -471,3 +471,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 6c1aba5d2027..031e6d3ebd4d 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..f99dc79ebb40 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");
@@ -185,16 +320,35 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
 {
 	int ret;
 
+	ret = apmf_check_smart_pc(dev);
+	if (ret) {
+		/*
+		 * 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).
+		 */
+		dev_info(dev->dev, "PMF Smart PC not advertised in BIOS!:%d\n", ret);
+		return -ENODEV;
+	}
+
 	ret = amd_pmf_tee_init(dev);
 	if (ret)
 		return ret;
 
 	INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
-	return 0;
+	amd_pmf_set_dram_addr(dev, true);
+	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);
 }
-- 
2.25.1


^ permalink raw reply related

* [PATCH v7 05/12] platform/x86/amd/pmf: change amd_pmf_init_features() call sequence
From: Shyam Sundar S K @ 2023-12-12  1:46 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K
In-Reply-To: <20231212014705.2017474-1-Shyam-sundar.S-k@amd.com>

To sideload pmf policy binaries, the Smart PC Solution Builder provides a
debugfs file called "update_policy"; that gets created under a new debugfs
directory called "pb" and this new directory has to be associated with
existing parent directory for PMF driver called "amd_pmf".

In the current code structure, amd_pmf_dbgfs_register() is called after
amd_pmf_init_features(). This will not help when the Smart PC builder
feature has to be assoicated to the parent directory.

Hence change the order of amd_pmf_dbgfs_register() and call it before
amd_pmf_init_features() so that when the Smart PC init happens, it has the
parent debugfs directory to get itself hooked.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index c10d40b33667..feaa09f5b35a 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -431,9 +431,9 @@ static int amd_pmf_probe(struct platform_device *pdev)
 
 	apmf_acpi_init(dev);
 	platform_set_drvdata(pdev, dev);
+	amd_pmf_dbgfs_register(dev);
 	amd_pmf_init_features(dev);
 	apmf_install_handler(dev);
-	amd_pmf_dbgfs_register(dev);
 
 	dev_info(dev->dev, "registered PMF device successfully\n");
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH v7 06/12] platform/x86/amd/pmf: Add support to get inputs from other subsystems
From: Shyam Sundar S K @ 2023-12-12  1:46 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K
In-Reply-To: <20231212014705.2017474-1-Shyam-sundar.S-k@amd.com>

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: Mario Limonciello <mario.limonciello@amd.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@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 031e6d3ebd4d..4da51eb28b6f 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 f99dc79ebb40..e96db406e91b 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);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v7 07/12] platform/x86/amd/pmf: Add support update p3t limit
From: Shyam Sundar S K @ 2023-12-12  1:47 UTC (permalink / raw)
  To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
	benjamin.tissoires
  Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
	Shyam Sundar S K
In-Reply-To: <20231212014705.2017474-1-Shyam-sundar.S-k@amd.com>

P3T (Peak Package Power Limit) is a metric within the SMU controller
that can influence the power limits. Add support from the driver
to update P3T limits accordingly.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/pmf.h    | 3 +++
 drivers/platform/x86/amd/pmf/tee-if.c | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 4da51eb28b6f..37bf1c701361 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -49,6 +49,7 @@
 #define GET_STT_MIN_LIMIT	0x1F
 #define GET_STT_LIMIT_APU	0x20
 #define GET_STT_LIMIT_HS2	0x21
+#define SET_P3T				0x23 /* P3T: Peak Package Power Limit */
 
 /* OS slider update notification */
 #define DC_BEST_PERF		0
@@ -72,6 +73,7 @@
 #define PMF_POLICY_STT_MIN					6
 #define PMF_POLICY_STT_SKINTEMP_APU				7
 #define PMF_POLICY_STT_SKINTEMP_HS2				8
+#define PMF_POLICY_P3T						38
 
 /* TA macros */
 #define PMF_TA_IF_VERSION_MAJOR				1
@@ -481,6 +483,7 @@ struct pmf_action_table {
 	u32 stt_minlimit;	/* in mW */
 	u32 stt_skintemp_apu;	/* in C */
 	u32 stt_skintemp_hs2;	/* in C */
+	u32 p3t_limit;		/* in mW */
 };
 
 /* Input conditions */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index e96db406e91b..bf8cb98d41ec 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -105,6 +105,14 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 				dev->prev_data->stt_skintemp_hs2 = val;
 			}
 			break;
+
+		case PMF_POLICY_P3T:
+			if (dev->prev_data->p3t_limit != val) {
+				amd_pmf_send_cmd(dev, SET_P3T, false, val, NULL);
+				dev_dbg(dev->dev, "update P3T: %u\n", val);
+				dev->prev_data->p3t_limit = val;
+			}
+			break;
 		}
 	}
 }
-- 
2.25.1


^ permalink raw reply related


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