* [PATCH v6 4/7] input/vmmouse: Use VMware hypercall API
From: Alexey Makhalov @ 2024-01-09 8:40 UTC (permalink / raw)
To: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx
Cc: x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov
In-Reply-To: <20240109084052.58661-1-amakhalov@vmware.com>
Switch from VMWARE_HYPERCALL macro to vmware_hypercall API.
Eliminate arch specific code. No functional changes intended.
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Nadav Amit <namit@vmware.com>
Reviewed-by: Zack Rusin <zackr@vmware.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/mouse/vmmouse.c | 78 ++++++++++-------------------------
1 file changed, 22 insertions(+), 56 deletions(-)
diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index ad94c835ee66..fb1d986a6895 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -21,19 +21,16 @@
#include "psmouse.h"
#include "vmmouse.h"
-#define VMMOUSE_PROTO_MAGIC 0x564D5868U
-
/*
* Main commands supported by the vmmouse hypervisor port.
*/
-#define VMMOUSE_PROTO_CMD_GETVERSION 10
-#define VMMOUSE_PROTO_CMD_ABSPOINTER_DATA 39
-#define VMMOUSE_PROTO_CMD_ABSPOINTER_STATUS 40
-#define VMMOUSE_PROTO_CMD_ABSPOINTER_COMMAND 41
-#define VMMOUSE_PROTO_CMD_ABSPOINTER_RESTRICT 86
+#define VMWARE_CMD_ABSPOINTER_DATA 39
+#define VMWARE_CMD_ABSPOINTER_STATUS 40
+#define VMWARE_CMD_ABSPOINTER_COMMAND 41
+#define VMWARE_CMD_ABSPOINTER_RESTRICT 86
/*
- * Subcommands for VMMOUSE_PROTO_CMD_ABSPOINTER_COMMAND
+ * Subcommands for VMWARE_CMD_ABSPOINTER_COMMAND
*/
#define VMMOUSE_CMD_ENABLE 0x45414552U
#define VMMOUSE_CMD_DISABLE 0x000000f5U
@@ -76,30 +73,6 @@ struct vmmouse_data {
char dev_name[128];
};
-/*
- * Hypervisor-specific bi-directional communication channel
- * implementing the vmmouse protocol. Should never execute on
- * bare metal hardware.
- */
-#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
-({ \
- unsigned long __dummy1, __dummy2; \
- __asm__ __volatile__ (VMWARE_HYPERCALL : \
- "=a"(out1), \
- "=b"(out2), \
- "=c"(out3), \
- "=d"(out4), \
- "=S"(__dummy1), \
- "=D"(__dummy2) : \
- [port] "i" (VMWARE_HYPERVISOR_PORT), \
- [mode] "m" (vmware_hypercall_mode), \
- "a"(VMMOUSE_PROTO_MAGIC), \
- "b"(in1), \
- "c"(VMMOUSE_PROTO_CMD_##cmd), \
- "d"(0) : \
- "memory"); \
-})
-
/**
* vmmouse_report_button - report button state on the correct input device
*
@@ -147,14 +120,12 @@ static psmouse_ret_t vmmouse_report_events(struct psmouse *psmouse)
struct input_dev *abs_dev = priv->abs_dev;
struct input_dev *pref_dev;
u32 status, x, y, z;
- u32 dummy1, dummy2, dummy3;
unsigned int queue_length;
unsigned int count = 255;
while (count--) {
/* See if we have motion data. */
- VMMOUSE_CMD(ABSPOINTER_STATUS, 0,
- status, dummy1, dummy2, dummy3);
+ status = vmware_hypercall1(VMWARE_CMD_ABSPOINTER_STATUS, 0);
if ((status & VMMOUSE_ERROR) == VMMOUSE_ERROR) {
psmouse_err(psmouse, "failed to fetch status data\n");
/*
@@ -174,7 +145,8 @@ static psmouse_ret_t vmmouse_report_events(struct psmouse *psmouse)
}
/* Now get it */
- VMMOUSE_CMD(ABSPOINTER_DATA, 4, status, x, y, z);
+ status = vmware_hypercall4(VMWARE_CMD_ABSPOINTER_DATA, 4,
+ &x, &y, &z);
/*
* And report what we've got. Prefer to report button
@@ -249,14 +221,10 @@ static psmouse_ret_t vmmouse_process_byte(struct psmouse *psmouse)
static void vmmouse_disable(struct psmouse *psmouse)
{
u32 status;
- u32 dummy1, dummy2, dummy3, dummy4;
-
- VMMOUSE_CMD(ABSPOINTER_COMMAND, VMMOUSE_CMD_DISABLE,
- dummy1, dummy2, dummy3, dummy4);
- VMMOUSE_CMD(ABSPOINTER_STATUS, 0,
- status, dummy1, dummy2, dummy3);
+ vmware_hypercall1(VMWARE_CMD_ABSPOINTER_COMMAND, VMMOUSE_CMD_DISABLE);
+ status = vmware_hypercall1(VMWARE_CMD_ABSPOINTER_STATUS, 0);
if ((status & VMMOUSE_ERROR) != VMMOUSE_ERROR)
psmouse_warn(psmouse, "failed to disable vmmouse device\n");
}
@@ -273,26 +241,24 @@ static void vmmouse_disable(struct psmouse *psmouse)
static int vmmouse_enable(struct psmouse *psmouse)
{
u32 status, version;
- u32 dummy1, dummy2, dummy3, dummy4;
/*
* Try enabling the device. If successful, we should be able to
* read valid version ID back from it.
*/
- VMMOUSE_CMD(ABSPOINTER_COMMAND, VMMOUSE_CMD_ENABLE,
- dummy1, dummy2, dummy3, dummy4);
+ vmware_hypercall1(VMWARE_CMD_ABSPOINTER_COMMAND, VMMOUSE_CMD_ENABLE);
/*
* See if version ID can be retrieved.
*/
- VMMOUSE_CMD(ABSPOINTER_STATUS, 0, status, dummy1, dummy2, dummy3);
+ status = vmware_hypercall1(VMWARE_CMD_ABSPOINTER_STATUS, 0);
if ((status & 0x0000ffff) == 0) {
psmouse_dbg(psmouse, "empty flags - assuming no device\n");
return -ENXIO;
}
- VMMOUSE_CMD(ABSPOINTER_DATA, 1 /* single item */,
- version, dummy1, dummy2, dummy3);
+ version = vmware_hypercall1(VMWARE_CMD_ABSPOINTER_DATA,
+ 1 /* single item */);
if (version != VMMOUSE_VERSION_ID) {
psmouse_dbg(psmouse, "Unexpected version value: %u vs %u\n",
(unsigned) version, VMMOUSE_VERSION_ID);
@@ -303,11 +269,11 @@ static int vmmouse_enable(struct psmouse *psmouse)
/*
* Restrict ioport access, if possible.
*/
- VMMOUSE_CMD(ABSPOINTER_RESTRICT, VMMOUSE_RESTRICT_CPL0,
- dummy1, dummy2, dummy3, dummy4);
+ vmware_hypercall1(VMWARE_CMD_ABSPOINTER_RESTRICT,
+ VMMOUSE_RESTRICT_CPL0);
- VMMOUSE_CMD(ABSPOINTER_COMMAND, VMMOUSE_CMD_REQUEST_ABSOLUTE,
- dummy1, dummy2, dummy3, dummy4);
+ vmware_hypercall1(VMWARE_CMD_ABSPOINTER_COMMAND,
+ VMMOUSE_CMD_REQUEST_ABSOLUTE);
return 0;
}
@@ -344,7 +310,7 @@ static bool vmmouse_check_hypervisor(void)
*/
int vmmouse_detect(struct psmouse *psmouse, bool set_properties)
{
- u32 response, version, dummy1, dummy2;
+ u32 response, version, type;
if (!vmmouse_check_hypervisor()) {
psmouse_dbg(psmouse,
@@ -353,9 +319,9 @@ int vmmouse_detect(struct psmouse *psmouse, bool set_properties)
}
/* Check if the device is present */
- response = ~VMMOUSE_PROTO_MAGIC;
- VMMOUSE_CMD(GETVERSION, 0, version, response, dummy1, dummy2);
- if (response != VMMOUSE_PROTO_MAGIC || version == 0xffffffffU)
+ response = ~VMWARE_HYPERVISOR_MAGIC;
+ version = vmware_hypercall3(VMWARE_CMD_GETVERSION, 0, &response, &type);
+ if (response != VMWARE_HYPERVISOR_MAGIC || version == 0xffffffffU)
return -ENXIO;
if (set_properties) {
--
2.39.0
^ permalink raw reply related
* [PATCH v6 5/7] drm/vmwgfx: Use VMware hypercall API
From: Alexey Makhalov @ 2024-01-09 8:40 UTC (permalink / raw)
To: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx
Cc: x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov
In-Reply-To: <20240109084052.58661-1-amakhalov@vmware.com>
Switch from VMWARE_HYPERCALL macro to vmware_hypercall API.
Eliminate arch specific code.
drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h: implement arm64 variant
of vmware_hypercall. And keep it here until introduction of ARM64
VMWare hypervisor interface.
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Nadav Amit <namit@vmware.com>
Reviewed-by: Zack Rusin <zackr@vmware.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 173 +++++++------------
drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h | 197 +++++++++++++++-------
drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h | 187 --------------------
3 files changed, 197 insertions(+), 360 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 2651fe0ef518..1f15990d3934 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -48,8 +48,6 @@
#define RETRIES 3
-#define VMW_HYPERVISOR_MAGIC 0x564D5868
-
#define VMW_PORT_CMD_MSG 30
#define VMW_PORT_CMD_HB_MSG 0
#define VMW_PORT_CMD_OPEN_CHANNEL (MSG_TYPE_OPEN << 16 | VMW_PORT_CMD_MSG)
@@ -104,20 +102,18 @@ static const char* const mksstat_kern_name_desc[MKSSTAT_KERN_COUNT][2] =
*/
static int vmw_open_channel(struct rpc_channel *channel, unsigned int protocol)
{
- unsigned long eax, ebx, ecx, edx, si = 0, di = 0;
+ u32 ecx, edx, esi, edi;
- VMW_PORT(VMW_PORT_CMD_OPEN_CHANNEL,
- (protocol | GUESTMSG_FLAG_COOKIE), si, di,
- 0,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall6(VMW_PORT_CMD_OPEN_CHANNEL,
+ (protocol | GUESTMSG_FLAG_COOKIE), 0,
+ &ecx, &edx, &esi, &edi);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0)
return -EINVAL;
channel->channel_id = HIGH_WORD(edx);
- channel->cookie_high = si;
- channel->cookie_low = di;
+ channel->cookie_high = esi;
+ channel->cookie_low = edi;
return 0;
}
@@ -133,17 +129,13 @@ static int vmw_open_channel(struct rpc_channel *channel, unsigned int protocol)
*/
static int vmw_close_channel(struct rpc_channel *channel)
{
- unsigned long eax, ebx, ecx, edx, si, di;
-
- /* Set up additional parameters */
- si = channel->cookie_high;
- di = channel->cookie_low;
+ u32 ecx;
- VMW_PORT(VMW_PORT_CMD_CLOSE_CHANNEL,
- 0, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall5(VMW_PORT_CMD_CLOSE_CHANNEL,
+ 0, channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ecx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0)
return -EINVAL;
@@ -163,24 +155,18 @@ static int vmw_close_channel(struct rpc_channel *channel)
static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
const char *msg, bool hb)
{
- unsigned long si, di, eax, ebx, ecx, edx;
+ u32 ebx, ecx;
unsigned long msg_len = strlen(msg);
/* HB port can't access encrypted memory. */
if (hb && !cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
- unsigned long bp = channel->cookie_high;
- u32 channel_id = (channel->channel_id << 16);
-
- si = (uintptr_t) msg;
- di = channel->cookie_low;
-
- VMW_PORT_HB_OUT(
+ vmware_hypercall_hb_out(
(MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG,
- msg_len, si, di,
- VMWARE_HYPERVISOR_HB | channel_id |
- VMWARE_HYPERVISOR_OUT,
- VMW_HYPERVISOR_MAGIC, bp,
- eax, ebx, ecx, edx, si, di);
+ msg_len,
+ channel->channel_id << 16,
+ (uintptr_t) msg, channel->cookie_low,
+ channel->cookie_high,
+ &ebx);
return ebx;
}
@@ -194,14 +180,13 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
memcpy(&word, msg, bytes);
msg_len -= bytes;
msg += bytes;
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_SENDPAYLOAD << 16),
- word, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+
+ vmware_hypercall5(VMW_PORT_CMD_MSG |
+ (MSG_TYPE_SENDPAYLOAD << 16),
+ word, channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ecx);
}
return ecx;
@@ -220,22 +205,17 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
unsigned long reply_len, bool hb)
{
- unsigned long si, di, eax, ebx, ecx, edx;
+ u32 ebx, ecx, edx;
/* HB port can't access encrypted memory */
if (hb && !cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
- unsigned long bp = channel->cookie_low;
- u32 channel_id = (channel->channel_id << 16);
-
- si = channel->cookie_high;
- di = (uintptr_t) reply;
-
- VMW_PORT_HB_IN(
+ vmware_hypercall_hb_in(
(MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG,
- reply_len, si, di,
- VMWARE_HYPERVISOR_HB | channel_id,
- VMW_HYPERVISOR_MAGIC, bp,
- eax, ebx, ecx, edx, si, di);
+ reply_len,
+ channel->channel_id << 16,
+ channel->cookie_high,
+ (uintptr_t) reply, channel->cookie_low,
+ &ebx);
return ebx;
}
@@ -245,14 +225,13 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
while (reply_len) {
unsigned int bytes = min_t(unsigned long, reply_len, 4);
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_RECVPAYLOAD << 16),
- MESSAGE_STATUS_SUCCESS, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall7(VMW_PORT_CMD_MSG |
+ (MSG_TYPE_RECVPAYLOAD << 16),
+ MESSAGE_STATUS_SUCCESS,
+ channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ebx, &ecx, &edx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0)
break;
@@ -276,22 +255,18 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
*/
static int vmw_send_msg(struct rpc_channel *channel, const char *msg)
{
- unsigned long eax, ebx, ecx, edx, si, di;
+ u32 ebx, ecx;
size_t msg_len = strlen(msg);
int retries = 0;
while (retries < RETRIES) {
retries++;
- /* Set up additional parameters */
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_SENDSIZE,
- msg_len, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall5(VMW_PORT_CMD_SENDSIZE,
+ msg_len, channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ecx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0) {
/* Expected success. Give up. */
@@ -329,7 +304,7 @@ STACK_FRAME_NON_STANDARD(vmw_send_msg);
static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
size_t *msg_len)
{
- unsigned long eax, ebx, ecx, edx, si, di;
+ u32 ebx, ecx, edx;
char *reply;
size_t reply_len;
int retries = 0;
@@ -341,15 +316,11 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
while (retries < RETRIES) {
retries++;
- /* Set up additional parameters */
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_RECVSIZE,
- 0, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall7(VMW_PORT_CMD_RECVSIZE,
+ 0, channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ebx, &ecx, &edx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0) {
DRM_ERROR("Failed to get reply size for host message.\n");
@@ -384,16 +355,12 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
reply[reply_len] = '\0';
-
- /* Ack buffer */
- si = channel->cookie_high;
- di = channel->cookie_low;
-
- VMW_PORT(VMW_PORT_CMD_RECVSTATUS,
- MESSAGE_STATUS_SUCCESS, si, di,
- channel->channel_id << 16,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall5(VMW_PORT_CMD_RECVSTATUS,
+ MESSAGE_STATUS_SUCCESS,
+ channel->channel_id << 16,
+ channel->cookie_high,
+ channel->cookie_low,
+ &ecx);
if ((HIGH_WORD(ecx) & MESSAGE_STATUS_SUCCESS) == 0) {
kfree(reply);
@@ -652,13 +619,7 @@ static inline void reset_ppn_array(PPN64 *arr, size_t size)
*/
static inline void hypervisor_ppn_reset_all(void)
{
- unsigned long eax, ebx, ecx, edx, si = 0, di = 0;
-
- VMW_PORT(VMW_PORT_CMD_MKSGS_RESET,
- 0, si, di,
- 0,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall1(VMW_PORT_CMD_MKSGS_RESET, 0);
}
/**
@@ -669,13 +630,7 @@ static inline void hypervisor_ppn_reset_all(void)
*/
static inline void hypervisor_ppn_add(PPN64 pfn)
{
- unsigned long eax, ebx, ecx, edx, si = 0, di = 0;
-
- VMW_PORT(VMW_PORT_CMD_MKSGS_ADD_PPN,
- (unsigned long)pfn, si, di,
- 0,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall1(VMW_PORT_CMD_MKSGS_ADD_PPN, (unsigned long)pfn);
}
/**
@@ -686,13 +641,7 @@ static inline void hypervisor_ppn_add(PPN64 pfn)
*/
static inline void hypervisor_ppn_remove(PPN64 pfn)
{
- unsigned long eax, ebx, ecx, edx, si = 0, di = 0;
-
- VMW_PORT(VMW_PORT_CMD_MKSGS_REMOVE_PPN,
- (unsigned long)pfn, si, di,
- 0,
- VMW_HYPERVISOR_MAGIC,
- eax, ebx, ecx, edx, si, di);
+ vmware_hypercall1(VMW_PORT_CMD_MKSGS_REMOVE_PPN, (unsigned long)pfn);
}
#if IS_ENABLED(CONFIG_DRM_VMWGFX_MKSSTATS)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h
index 4f40167ad61f..29bd0af83038 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h
@@ -34,6 +34,8 @@
#define VMWARE_HYPERVISOR_HB BIT(0)
#define VMWARE_HYPERVISOR_OUT BIT(1)
+#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
+
#define X86_IO_MAGIC 0x86
#define X86_IO_W7_SIZE_SHIFT 0
@@ -45,86 +47,159 @@
#define X86_IO_W7_IMM_SHIFT 5
#define X86_IO_W7_IMM_MASK (0xff << X86_IO_W7_IMM_SHIFT)
-static inline void vmw_port(unsigned long cmd, unsigned long in_ebx,
- unsigned long in_si, unsigned long in_di,
- unsigned long flags, unsigned long magic,
- unsigned long *eax, unsigned long *ebx,
- unsigned long *ecx, unsigned long *edx,
- unsigned long *si, unsigned long *di)
+static inline
+unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
{
- register u64 x0 asm("x0") = magic;
- register u64 x1 asm("x1") = in_ebx;
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
+ register u64 x1 asm("x1") = in1;
register u64 x2 asm("x2") = cmd;
- register u64 x3 asm("x3") = flags | VMWARE_HYPERVISOR_PORT;
- register u64 x4 asm("x4") = in_si;
- register u64 x5 asm("x5") = in_di;
+ register u64 x3 asm("x3") = VMWARE_HYPERVISOR_PORT;
+ register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
+ X86_IO_W7_WITH |
+ X86_IO_W7_DIR |
+ (2 << X86_IO_W7_SIZE_SHIFT);
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0)
+ : "r" (x1), "r" (x2), "r" (x3), "r" (x7)
+ : "memory");
+
+ return x0;
+}
+
+static inline
+unsigned long vmware_hypercall5(unsigned long cmd, unsigned long in1,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, uint32_t *out2)
+{
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
+ register u64 x1 asm("x1") = in1;
+ register u64 x2 asm("x2") = cmd;
+ register u64 x3 asm("x3") = in3 | VMWARE_HYPERVISOR_PORT;
+ register u64 x4 asm("x4") = in4;
+ register u64 x5 asm("x5") = in5;
register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
X86_IO_W7_WITH |
X86_IO_W7_DIR |
(2 << X86_IO_W7_SIZE_SHIFT);
- asm volatile("mrs xzr, mdccsr_el0 \n\t"
- : "+r"(x0), "+r"(x1), "+r"(x2),
- "+r"(x3), "+r"(x4), "+r"(x5)
- : "r"(x7)
- :);
- *eax = x0;
- *ebx = x1;
- *ecx = x2;
- *edx = x3;
- *si = x4;
- *di = x5;
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0), "+r" (x2)
+ : "r" (x1), "r" (x3), "r" (x4), "r" (x5), "r" (x7)
+ : "memory");
+
+ *out2 = x2;
+ return x0;
}
-static inline void vmw_port_hb(unsigned long cmd, unsigned long in_ecx,
- unsigned long in_si, unsigned long in_di,
- unsigned long flags, unsigned long magic,
- unsigned long bp, u32 w7dir,
- unsigned long *eax, unsigned long *ebx,
- unsigned long *ecx, unsigned long *edx,
- unsigned long *si, unsigned long *di)
+static inline
+unsigned long vmware_hypercall6(unsigned long cmd, unsigned long in1,
+ unsigned long in3, uint32_t *out2,
+ uint32_t *out3, uint32_t *out4,
+ uint32_t *out5)
{
- register u64 x0 asm("x0") = magic;
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
+ register u64 x1 asm("x1") = in1;
+ register u64 x2 asm("x2") = cmd;
+ register u64 x3 asm("x3") = in3 | VMWARE_HYPERVISOR_PORT;
+ register u64 x4 asm("x4");
+ register u64 x5 asm("x5");
+ register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
+ X86_IO_W7_WITH |
+ X86_IO_W7_DIR |
+ (2 << X86_IO_W7_SIZE_SHIFT);
+
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0), "+r" (x2), "+r" (x3), "=r" (x4), "=r" (x5)
+ : "r" (x1), "r" (x7)
+ : "memory");
+
+ *out2 = x2;
+ *out3 = x3;
+ *out4 = x4;
+ *out5 = x5;
+ return x0;
+}
+
+static inline
+unsigned long vmware_hypercall7(unsigned long cmd, unsigned long in1,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, uint32_t *out1,
+ uint32_t *out2, uint32_t *out3)
+{
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
+ register u64 x1 asm("x1") = in1;
+ register u64 x2 asm("x2") = cmd;
+ register u64 x3 asm("x3") = in3 | VMWARE_HYPERVISOR_PORT;
+ register u64 x4 asm("x4") = in4;
+ register u64 x5 asm("x5") = in5;
+ register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
+ X86_IO_W7_WITH |
+ X86_IO_W7_DIR |
+ (2 << X86_IO_W7_SIZE_SHIFT);
+
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0), "+r" (x1), "+r" (x2), "+r" (x3)
+ : "r" (x4), "r" (x5), "r" (x7)
+ : "memory");
+
+ *out1 = x1;
+ *out2 = x2;
+ *out3 = x3;
+ return x0;
+}
+
+static inline
+unsigned long vmware_hypercall_hb(unsigned long cmd, unsigned long in2,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, unsigned long in6,
+ uint32_t *out1, int dir)
+{
+ register u64 x0 asm("x0") = VMWARE_HYPERVISOR_MAGIC;
register u64 x1 asm("x1") = cmd;
- register u64 x2 asm("x2") = in_ecx;
- register u64 x3 asm("x3") = flags | VMWARE_HYPERVISOR_PORT_HB;
- register u64 x4 asm("x4") = in_si;
- register u64 x5 asm("x5") = in_di;
- register u64 x6 asm("x6") = bp;
+ register u64 x2 asm("x2") = in2;
+ register u64 x3 asm("x3") = in3 | VMWARE_HYPERVISOR_PORT_HB;
+ register u64 x4 asm("x4") = in4;
+ register u64 x5 asm("x5") = in5;
+ register u64 x6 asm("x6") = in6;
register u64 x7 asm("x7") = ((u64)X86_IO_MAGIC << 32) |
X86_IO_W7_STR |
X86_IO_W7_WITH |
- w7dir;
-
- asm volatile("mrs xzr, mdccsr_el0 \n\t"
- : "+r"(x0), "+r"(x1), "+r"(x2),
- "+r"(x3), "+r"(x4), "+r"(x5)
- : "r"(x6), "r"(x7)
- :);
- *eax = x0;
- *ebx = x1;
- *ecx = x2;
- *edx = x3;
- *si = x4;
- *di = x5;
-}
+ dir;
-#define VMW_PORT(cmd, in_ebx, in_si, in_di, flags, magic, eax, ebx, ecx, edx, \
- si, di) \
- vmw_port(cmd, in_ebx, in_si, in_di, flags, magic, &eax, &ebx, &ecx, \
- &edx, &si, &di)
+ asm_inline volatile (
+ "mrs xzr, mdccsr_el0; "
+ : "+r" (x0), "+r" (x1)
+ : "r" (x2), "r" (x3), "r" (x4), "r" (x5),
+ "r" (x6), "r" (x7)
+ : "memory");
-#define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di, flags, magic, bp, eax, ebx, \
- ecx, edx, si, di) \
- vmw_port_hb(cmd, in_ecx, in_si, in_di, flags, magic, bp, \
- 0, &eax, &ebx, &ecx, &edx, &si, &di)
+ *out1 = x1;
+ return x0;
+}
-#define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di, flags, magic, bp, eax, ebx, \
- ecx, edx, si, di) \
- vmw_port_hb(cmd, in_ecx, in_si, in_di, flags, magic, bp, \
- X86_IO_W7_DIR, &eax, &ebx, &ecx, &edx, &si, &di)
+static inline
+unsigned long vmware_hypercall_hb_out(unsigned long cmd, unsigned long in2,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, unsigned long in6,
+ uint32_t *out1)
+{
+ return vmware_hypercall_hb(cmd, in2, in3, in4, in5, in6, out1, 0);
+}
+static inline
+unsigned long vmware_hypercall_hb_in(unsigned long cmd, unsigned long in2,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5, unsigned long in6,
+ uint32_t *out1)
+{
+ return vmware_hypercall_hb(cmd, in2, in3, in4, in5, in6, out1,
+ X86_IO_W7_DIR);
+}
#endif
#endif /* _VMWGFX_MSG_ARM64_H */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h
index e040ee21ea1a..13304d34cc6e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h
@@ -37,193 +37,6 @@
#include <asm/vmware.h>
-/**
- * Hypervisor-specific bi-directional communication channel. Should never
- * execute on bare metal hardware. The caller must make sure to check for
- * supported hypervisor before using these macros.
- *
- * The last two parameters are both input and output and must be initialized.
- *
- * @cmd: [IN] Message Cmd
- * @in_ebx: [IN] Message Len, through EBX
- * @in_si: [IN] Input argument through SI, set to 0 if not used
- * @in_di: [IN] Input argument through DI, set ot 0 if not used
- * @flags: [IN] hypercall flags + [channel id]
- * @magic: [IN] hypervisor magic value
- * @eax: [OUT] value of EAX register
- * @ebx: [OUT] e.g. status from an HB message status command
- * @ecx: [OUT] e.g. status from a non-HB message status command
- * @edx: [OUT] e.g. channel id
- * @si: [OUT]
- * @di: [OUT]
- */
-#define VMW_PORT(cmd, in_ebx, in_si, in_di, \
- flags, magic, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile (VMWARE_HYPERCALL : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- [port] "i" (VMWARE_HYPERVISOR_PORT), \
- [mode] "m" (vmware_hypercall_mode), \
- "a"(magic), \
- "b"(in_ebx), \
- "c"(cmd), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di) : \
- "memory"); \
-})
-
-
-/**
- * Hypervisor-specific bi-directional communication channel. Should never
- * execute on bare metal hardware. The caller must make sure to check for
- * supported hypervisor before using these macros.
- *
- * The last 3 parameters are both input and output and must be initialized.
- *
- * @cmd: [IN] Message Cmd
- * @in_ecx: [IN] Message Len, through ECX
- * @in_si: [IN] Input argument through SI, set to 0 if not used
- * @in_di: [IN] Input argument through DI, set to 0 if not used
- * @flags: [IN] hypercall flags + [channel id]
- * @magic: [IN] hypervisor magic value
- * @bp: [IN]
- * @eax: [OUT] value of EAX register
- * @ebx: [OUT] e.g. status from an HB message status command
- * @ecx: [OUT] e.g. status from a non-HB message status command
- * @edx: [OUT] e.g. channel id
- * @si: [OUT]
- * @di: [OUT]
- */
-#ifdef __x86_64__
-
-#define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di, \
- flags, magic, bp, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile ( \
- UNWIND_HINT_SAVE \
- "push %%rbp;" \
- UNWIND_HINT_UNDEFINED \
- "mov %12, %%rbp;" \
- "rep outsb;" \
- "pop %%rbp;" \
- UNWIND_HINT_RESTORE : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- "a"(magic), \
- "b"(cmd), \
- "c"(in_ecx), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di), \
- "r"(bp) : \
- "memory", "cc"); \
-})
-
-
-#define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di, \
- flags, magic, bp, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile ( \
- UNWIND_HINT_SAVE \
- "push %%rbp;" \
- UNWIND_HINT_UNDEFINED \
- "mov %12, %%rbp;" \
- "rep insb;" \
- "pop %%rbp;" \
- UNWIND_HINT_RESTORE : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- "a"(magic), \
- "b"(cmd), \
- "c"(in_ecx), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di), \
- "r"(bp) : \
- "memory", "cc"); \
-})
-
-#elif defined(__i386__)
-
-/*
- * In the 32-bit version of this macro, we store bp in a memory location
- * because we've ran out of registers.
- * Now we can't reference that memory location while we've modified
- * %esp or %ebp, so we first push it on the stack, just before we push
- * %ebp, and then when we need it we read it from the stack where we
- * just pushed it.
- */
-#define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di, \
- flags, magic, bp, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile ("push %12;" \
- "push %%ebp;" \
- "mov 0x04(%%esp), %%ebp;" \
- VMWARE_HYPERCALL_HB_OUT \
- "pop %%ebp;" \
- "add $0x04, %%esp;" : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- "a"(magic), \
- "b"(cmd), \
- "c"(in_ecx), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di), \
- "m"(bp) : \
- "memory", "cc"); \
-})
-
-
-#define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di, \
- flags, magic, bp, \
- eax, ebx, ecx, edx, si, di) \
-({ \
- asm volatile ("push %12;" \
- "push %%ebp;" \
- "mov 0x04(%%esp), %%ebp;" \
- VMWARE_HYPERCALL_HB_IN \
- "pop %%ebp;" \
- "add $0x04, %%esp;" : \
- "=a"(eax), \
- "=b"(ebx), \
- "=c"(ecx), \
- "=d"(edx), \
- "=S"(si), \
- "=D"(di) : \
- "a"(magic), \
- "b"(cmd), \
- "c"(in_ecx), \
- "d"(flags), \
- "S"(in_si), \
- "D"(in_di), \
- "m"(bp) : \
- "memory", "cc"); \
-})
-#endif /* defined(__i386__) */
-
#endif /* defined(__i386__) || defined(__x86_64__) */
#endif /* _VMWGFX_MSG_X86_H */
--
2.39.0
^ permalink raw reply related
* [PATCH v6 6/7] x86/vmware: Undefine VMWARE_HYPERCALL
From: Alexey Makhalov @ 2024-01-09 8:40 UTC (permalink / raw)
To: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx
Cc: x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov
In-Reply-To: <20240109084052.58661-1-amakhalov@vmware.com>
No more direct use of VMWARE_HYPERCALL macro should be allowed.
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
---
arch/x86/include/asm/vmware.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
index 2ac87068184a..84a31f579a30 100644
--- a/arch/x86/include/asm/vmware.h
+++ b/arch/x86/include/asm/vmware.h
@@ -273,5 +273,6 @@ unsigned long vmware_hypercall_hb_in(unsigned long cmd, unsigned long in2,
}
#undef VMW_BP_REG
#undef VMW_BP_CONSTRAINT
+#undef VMWARE_HYPERCALL
#endif
--
2.39.0
^ permalink raw reply related
* [PATCH v6 7/7] x86/vmware: Add TDX hypercall support
From: Alexey Makhalov @ 2024-01-09 8:40 UTC (permalink / raw)
To: linux-kernel, virtualization, bp, hpa, dave.hansen, mingo, tglx
Cc: x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov
In-Reply-To: <20240109084052.58661-1-amakhalov@vmware.com>
VMware hypercalls use I/O port, VMCALL or VMMCALL instructions.
Add __tdx_hypercall path to support TDX guests.
No change in high bandwidth hypercalls, as only low bandwidth
ones are supported for TDX guests.
Co-developed-by: Tim Merrifield <timothym@vmware.com>
Signed-off-by: Tim Merrifield <timothym@vmware.com>
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Nadav Amit <namit@vmware.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
arch/x86/include/asm/vmware.h | 79 +++++++++++++++++++++++++++++++++++
arch/x86/kernel/cpu/vmware.c | 25 +++++++++++
2 files changed, 104 insertions(+)
diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
index 84a31f579a30..3bd593c6591d 100644
--- a/arch/x86/include/asm/vmware.h
+++ b/arch/x86/include/asm/vmware.h
@@ -18,6 +18,12 @@
* arg2 - Hypercall command
* arg3 bits [15:0] - Port number, LB and direction flags
*
+ * - Low bandwidth TDX hypercalls (x86_64 only) are similar to LB
+ * hypercalls. They also have up to 6 input and 6 output on registers
+ * arguments, with different argument to register mapping:
+ * %r12 (arg0), %rbx (arg1), %r13 (arg2), %rdx (arg3),
+ * %rsi (arg4), %rdi (arg5).
+ *
* - High bandwidth (HB) hypercalls are I/O port based only. They have
* up to 7 input and 7 output arguments passed and returned using
* registers: %eax (arg0), %ebx (arg1), %ecx (arg2), %edx (arg3),
@@ -54,12 +60,61 @@
#define VMWARE_CMD_GETHZ 45
#define VMWARE_CMD_GETVCPU_INFO 68
#define VMWARE_CMD_STEALCLOCK 91
+/*
+ * Hypercall command mask:
+ * bits [6:0] command, range [0, 127]
+ * bits [19:16] sub-command, range [0, 15]
+ */
+#define VMWARE_CMD_MASK 0xf007fU
#define CPUID_VMWARE_FEATURES_ECX_VMMCALL BIT(0)
#define CPUID_VMWARE_FEATURES_ECX_VMCALL BIT(1)
extern u8 vmware_hypercall_mode;
+#define VMWARE_TDX_VENDOR_LEAF 0x1af7e4909ULL
+#define VMWARE_TDX_HCALL_FUNC 1
+
+extern unsigned long vmware_tdx_hypercall(unsigned long cmd,
+ struct tdx_module_args *args);
+
+/*
+ * TDCALL[TDG.VP.VMCALL] uses %rax (arg0) and %rcx (arg2). Therefore,
+ * we remap those registers to %r12 and %r13, respectively.
+ */
+static inline
+unsigned long vmware_tdx_hypercall_args(unsigned long cmd, unsigned long in1,
+ unsigned long in3, unsigned long in4,
+ unsigned long in5,
+ uint32_t *out1, uint32_t *out2,
+ uint32_t *out3, uint32_t *out4,
+ uint32_t *out5)
+{
+ unsigned long ret;
+
+ struct tdx_module_args args = {
+ .rbx = in1,
+ .rdx = in3,
+ .rsi = in4,
+ .rdi = in5,
+ };
+
+ ret = vmware_tdx_hypercall(cmd, &args);
+
+ if (out1)
+ *out1 = args.rbx;
+ if (out2)
+ *out2 = args.r13;
+ if (out3)
+ *out3 = args.rdx;
+ if (out4)
+ *out4 = args.rsi;
+ if (out5)
+ *out5 = args.rdi;
+
+ return ret;
+}
+
/*
* The low bandwidth call. The low word of %edx is presumed to have OUT bit
* set. The high word of %edx may contain input data from the caller.
@@ -87,6 +142,10 @@ unsigned long vmware_hypercall1(unsigned long cmd, unsigned long in1)
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, 0, 0, 0,
+ NULL, NULL, NULL, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
@@ -105,6 +164,10 @@ unsigned long vmware_hypercall3(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, 0, 0, 0,
+ out1, out2, NULL, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=b" (*out1), "=c" (*out2)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
@@ -124,6 +187,10 @@ unsigned long vmware_hypercall4(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, 0, 0, 0,
+ out1, out2, out3, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=b" (*out1), "=c" (*out2), "=d" (*out3)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
@@ -143,6 +210,10 @@ unsigned long vmware_hypercall5(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, in3, in4, in5,
+ NULL, out2, NULL, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=c" (*out2)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
@@ -165,6 +236,10 @@ unsigned long vmware_hypercall6(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, in3, 0, 0,
+ NULL, out2, out3, out4, out5);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=c" (*out2), "=d" (*out3), "=S" (*out4),
"=D" (*out5)
@@ -186,6 +261,10 @@ unsigned long vmware_hypercall7(unsigned long cmd, unsigned long in1,
{
unsigned long out0;
+ if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
+ return vmware_tdx_hypercall_args(cmd, in1, in3, in4, in5,
+ out1, out2, out3, NULL, NULL);
+
asm_inline volatile (VMWARE_HYPERCALL
: "=a" (out0), "=b" (*out1), "=c" (*out2), "=d" (*out3)
: [port] "i" (VMWARE_HYPERVISOR_PORT),
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 3aa1adaed18f..84caa67d4820 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -428,6 +428,31 @@ static bool __init vmware_legacy_x2apic_available(void)
(eax & BIT(VCPU_LEGACY_X2APIC));
}
+#ifdef CONFIG_INTEL_TDX_GUEST
+unsigned long vmware_tdx_hypercall(unsigned long cmd,
+ struct tdx_module_args *args)
+{
+ if (!hypervisor_is_type(X86_HYPER_VMWARE))
+ return ULONG_MAX;
+
+ if (cmd & ~VMWARE_CMD_MASK) {
+ pr_warn_once("Out of range command %lx\n", cmd);
+ return ULONG_MAX;
+ }
+
+ args->r10 = VMWARE_TDX_VENDOR_LEAF;
+ args->r11 = VMWARE_TDX_HCALL_FUNC;
+ args->r12 = VMWARE_HYPERVISOR_MAGIC;
+ args->r13 = cmd;
+ args->r15 = 0; /* CPL */
+
+ __tdx_hypercall(args);
+
+ return args->r12;
+}
+EXPORT_SYMBOL_GPL(vmware_tdx_hypercall);
+#endif
+
#ifdef CONFIG_AMD_MEM_ENCRYPT
static void vmware_sev_es_hcall_prepare(struct ghcb *ghcb,
struct pt_regs *regs)
--
2.39.0
^ permalink raw reply related
* Re: [PATCH v6 0/7] VMware hypercalls enhancements
From: Borislav Petkov @ 2024-01-09 10:21 UTC (permalink / raw)
To: Alexey Makhalov
Cc: linux-kernel, virtualization, hpa, dave.hansen, mingo, tglx, x86,
netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms, kirill.shutemov
In-Reply-To: <20240109084052.58661-1-amakhalov@vmware.com>
On Tue, Jan 09, 2024 at 12:40:45AM -0800, Alexey Makhalov wrote:
> v5->v6 change:
> - Added ack by Kirill A. Shutemov in patch 7.
Please do not spam. Adding someone's Ack does not mean you have to
resend the whole thing immediately again.
While waiting, please read Documentation/process/submitting-patches.rst
and especially:
"Don't get discouraged - or impatient
------------------------------------
After you have submitted your change, be patient and wait. Reviewers are
busy people and may not get to your patch right away.
Once upon a time, patches used to disappear into the void without comment,
but the development process works more smoothly than that now. You should
receive comments within a few weeks (typically 2-3); if that does not
happen, make sure that you have sent your patches to the right place.
Wait for a minimum of one week before resubmitting or pinging reviewers
- possibly longer during busy times like merge windows."
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH AUTOSEL 4.19 4/7] Input: amimouse - convert to platform remove callback returning void
From: Pavel Machek @ 2024-01-09 11:44 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, Uwe Kleine-König, Dmitry Torokhov,
linux-input
In-Reply-To: <20231226002649.7290-4-sashal@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
Hi!
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> [ Upstream commit 42b8ff47720258d1f6a4412e780a480c139773a0 ]
>
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is ignored (apart
> from emitting a warning) and this typically results in resource leaks.
>
> To improve here there is a quest to make the remove callback return
> void. In the first step of this quest all drivers are converted to
> .remove_new(), which already returns void. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
>
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
We don't really need this for -stable.
Best regards,
Pavel
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Link: https://lore.kernel.org/r/20231201133747.1099286-2-u.kleine-koenig@pengutronix.de
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
--
People of Russia, stop Putin before his war on Ukraine escalates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Flood of logitech-hidpp-device messages in v6.7
From: Oleksandr Natalenko @ 2024-01-09 11:45 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires, Hans de Goede
[-- Attachment #1: Type: text/plain, Size: 2529 bytes --]
Hello Hans et al.
Starting from v6.7 release I get the following messages repeating in `dmesg` regularly:
```
Jan 09 10:05:06 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
Jan 09 10:07:15 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
Jan 09 10:16:51 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
Jan 09 10:16:55 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
Jan 09 10:16:55 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
Jan 09 10:36:31 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
Jan 09 10:37:07 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
Jan 09 10:46:21 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
Jan 09 10:48:23 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
Jan 09 11:12:27 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
Jan 09 11:12:47 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
Jan 09 11:12:47 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
Jan 09 11:38:32 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
Jan 09 11:43:32 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
Jan 09 11:45:10 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
Jan 09 11:45:11 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
Jan 09 11:45:11 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
Jan 09 12:31:48 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
Jan 09 12:33:21 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
```
I've got the following hardware:
* Bus 006 Device 004: ID 046d:c52b Logitech, Inc. Unifying Receiver
* Logitech MX Keys
* Logitech M510v2
With v6.6 I do not get those messages.
I think this is related to 680ee411a98e ("HID: logitech-hidpp: Fix connect event race").
My speculation is that some of the devices enter powersaving state after being idle for some time (5 mins?), and then wake up and reconnect once I touch either keyboard or mouse. I should highlight that everything works just fine, it is the flood of messages that worries me.
Is it expected?
Thank you.
--
Oleksandr Natalenko (post-factum)
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: Flood of logitech-hidpp-device messages in v6.7
From: Hans de Goede @ 2024-01-09 11:58 UTC (permalink / raw)
To: Oleksandr Natalenko, linux-kernel
Cc: linux-input, Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
In-Reply-To: <3277085.44csPzL39Z@natalenko.name>
Hi Oleksandr,
On 1/9/24 12:45, Oleksandr Natalenko wrote:
> Hello Hans et al.
>
> Starting from v6.7 release I get the following messages repeating in `dmesg` regularly:
>
> ```
> Jan 09 10:05:06 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> Jan 09 10:07:15 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
> Jan 09 10:16:51 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> Jan 09 10:16:55 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> Jan 09 10:16:55 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> Jan 09 10:36:31 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> Jan 09 10:37:07 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> Jan 09 10:46:21 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> Jan 09 10:48:23 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
> Jan 09 11:12:27 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> Jan 09 11:12:47 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> Jan 09 11:12:47 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> Jan 09 11:38:32 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> Jan 09 11:43:32 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
> Jan 09 11:45:10 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> Jan 09 11:45:11 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> Jan 09 11:45:11 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> Jan 09 12:31:48 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> Jan 09 12:33:21 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> ```
>
> I've got the following hardware:
>
> * Bus 006 Device 004: ID 046d:c52b Logitech, Inc. Unifying Receiver
> * Logitech MX Keys
> * Logitech M510v2
>
> With v6.6 I do not get those messages.
>
> I think this is related to 680ee411a98e ("HID: logitech-hidpp: Fix connect event race").
>
> My speculation is that some of the devices enter powersaving state after being idle for some time (5 mins?), and then wake up and reconnect once I touch either keyboard or mouse. I should highlight that everything works just fine, it is the flood of messages that worries me.
>
> Is it expected?
Yes this is expected, looking at your logs I see about 10 messages per
hour which IMHO is not that bad.
I guess we could change things to track we have logged the connect
message once and if yes then log future connect messages (and all
disconnect messages) at debug level.
Jiri, Benjamin, do you have any opinion on this ?
Regards,
Hans
^ permalink raw reply
* Re: Flood of logitech-hidpp-device messages in v6.7
From: Benjamin Tissoires @ 2024-01-09 14:18 UTC (permalink / raw)
To: Hans de Goede
Cc: Oleksandr Natalenko, linux-kernel, linux-input, Filipe Laíns,
Bastien Nocera, Jiri Kosina
In-Reply-To: <824573bb-ae01-41b9-8f97-a760ae8f3f18@redhat.com>
On Tue, Jan 9, 2024 at 12:58 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Oleksandr,
>
> On 1/9/24 12:45, Oleksandr Natalenko wrote:
> > Hello Hans et al.
> >
> > Starting from v6.7 release I get the following messages repeating in `dmesg` regularly:
> >
> > ```
> > Jan 09 10:05:06 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 10:07:15 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
> > Jan 09 10:16:51 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > Jan 09 10:16:55 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 10:16:55 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 10:36:31 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 10:37:07 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > Jan 09 10:46:21 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 10:48:23 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
> > Jan 09 11:12:27 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > Jan 09 11:12:47 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 11:12:47 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 11:38:32 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 11:43:32 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: Disconnected
> > Jan 09 11:45:10 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > Jan 09 11:45:11 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 11:45:11 spock kernel: logitech-hidpp-device 0003:046D:408A.0005: HID++ 4.5 device connected.
> > Jan 09 12:31:48 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: Disconnected
> > Jan 09 12:33:21 spock kernel: logitech-hidpp-device 0003:046D:4051.0006: HID++ 4.5 device connected.
> > ```
> >
> > I've got the following hardware:
> >
> > * Bus 006 Device 004: ID 046d:c52b Logitech, Inc. Unifying Receiver
> > * Logitech MX Keys
> > * Logitech M510v2
> >
> > With v6.6 I do not get those messages.
> >
> > I think this is related to 680ee411a98e ("HID: logitech-hidpp: Fix connect event race").
> >
> > My speculation is that some of the devices enter powersaving state after being idle for some time (5 mins?), and then wake up and reconnect once I touch either keyboard or mouse. I should highlight that everything works just fine, it is the flood of messages that worries me.
> >
> > Is it expected?
>
> Yes this is expected, looking at your logs I see about 10 messages per
> hour which IMHO is not that bad.
>
> I guess we could change things to track we have logged the connect
> message once and if yes then log future connect messages (and all
> disconnect messages) at debug level.
Sounds reasonable to me.
Cheers,
Benjamin
>
>
> Jiri, Benjamin, do you have any opinion on this ?
>
> Regards,
>
> Hans
>
>
>
^ permalink raw reply
* [PATCH v3 0/4] Add support of color temperature and chromaticity
From: Srinivas Pandruvada @ 2024-01-09 18:00 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada
The original series submitted to 6.7 (before revert) is modified to
solve regression issues on several platforms. There are two changes
introduced before adding support for new features to allow dynamic
addition of channels.
v3:
Addressed comments for v2, details in each patch.
v2:
New change to add channels dynamically
Modified color temperature and chromaticity to skip in case
of failures
Basavaraj Natikar (2):
iio: hid-sensor-als: Add light color temperature support
iio: hid-sensor-als: Add light chromaticity support
Srinivas Pandruvada (2):
iio: hid-sensor-als: Assign channels dynamically
iio: hid-sensor-als: Remove hardcoding of values for enums
drivers/iio/light/hid-sensor-als.c | 130 ++++++++++++++++++++++++-----
include/linux/hid-sensor-ids.h | 4 +
2 files changed, 115 insertions(+), 19 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v3 1/4] iio: hid-sensor-als: Assign channels dynamically
From: Srinivas Pandruvada @ 2024-01-09 18:00 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20240109180007.3373784-1-srinivas.pandruvada@linux.intel.com>
Instead of assuming that every channel defined statically by
als_channels[] is present, assign dynamically based on presence of the
respective usage id in the descriptor. This will allow to register ALS
with limited channel support. Append the timestamp as the last channel.
When not all usage ids are present, the scan index is adjusted to
exclude unsupported channels.
There is no intentional function changes done.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
v3:
Addressed comments from Jonthan:
- Remove channel allocation and move to iio_priv()
- Parse all usage IDs in a single loop and continue
for failure. This way the temperature and chromaticity
will not need any special processing to parse usage ids.
- Don't leave empty channel indexes
v2:
New change
drivers/iio/light/hid-sensor-als.c | 56 +++++++++++++++++++++---------
1 file changed, 39 insertions(+), 17 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 5cd27f04b45e..72a7c01c97f8 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -25,17 +25,26 @@ struct als_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info als[CHANNEL_SCAN_INDEX_MAX];
+ struct iio_chan_spec channels[CHANNEL_SCAN_INDEX_MAX + 1];
struct {
u32 illum[CHANNEL_SCAN_INDEX_MAX];
+ u32 scan_index[CHANNEL_SCAN_INDEX_MAX];
u64 timestamp __aligned(8);
} scan;
int scale_pre_decml;
int scale_post_decml;
int scale_precision;
int value_offset;
+ int num_channels;
s64 timestamp;
};
+/* The order of usage ids must match scan index starting from CHANNEL_SCAN_INDEX_INTENSITY */
+static const u32 als_usage_ids[] = {
+ HID_USAGE_SENSOR_LIGHT_ILLUM,
+ HID_USAGE_SENSOR_LIGHT_ILLUM,
+};
+
static const u32 als_sensitivity_addresses[] = {
HID_USAGE_SENSOR_DATA_LIGHT,
HID_USAGE_SENSOR_LIGHT_ILLUM,
@@ -216,11 +225,14 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
struct als_state *als_state = iio_priv(indio_dev);
int ret = -EINVAL;
u32 sample_data = *(u32 *)raw_data;
+ int scan_index;
switch (usage_id) {
case HID_USAGE_SENSOR_LIGHT_ILLUM:
- als_state->scan.illum[CHANNEL_SCAN_INDEX_INTENSITY] = sample_data;
- als_state->scan.illum[CHANNEL_SCAN_INDEX_ILLUM] = sample_data;
+ scan_index = als_state->scan.scan_index[CHANNEL_SCAN_INDEX_INTENSITY];
+ als_state->scan.illum[scan_index] = sample_data;
+ scan_index = als_state->scan.scan_index[CHANNEL_SCAN_INDEX_ILLUM];
+ als_state->scan.illum[scan_index] = sample_data;
ret = 0;
break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
@@ -237,27 +249,39 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
/* Parse report which is specific to an usage id*/
static int als_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
- struct iio_chan_spec *channels,
unsigned usage_id,
struct als_state *st)
{
- int ret;
+ struct iio_chan_spec *channels;
+ int ret, index = 0;
int i;
+ channels = st->channels;
+
for (i = 0; i <= CHANNEL_SCAN_INDEX_ILLUM; ++i) {
ret = sensor_hub_input_get_attribute_info(hsdev,
HID_INPUT_REPORT,
usage_id,
- HID_USAGE_SENSOR_LIGHT_ILLUM,
+ als_usage_ids[i],
&st->als[i]);
if (ret < 0)
- return ret;
- als_adjust_channel_bit_mask(channels, i, st->als[i].size);
+ continue;
+
+ channels[index] = als_channels[i];
+ st->scan.scan_index[i] = index;
+
+ als_adjust_channel_bit_mask(channels, index, st->als[i].size);
+ ++index;
dev_dbg(&pdev->dev, "als %x:%x\n", st->als[i].index,
st->als[i].report_id);
}
+ st->num_channels = index;
+ /* Return success even if one usage id is present */
+ if (index)
+ ret = 0;
+
st->scale_precision = hid_sensor_format_scale(usage_id,
&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
@@ -293,15 +317,7 @@ static int hid_als_probe(struct platform_device *pdev)
return ret;
}
- indio_dev->channels = devm_kmemdup(&pdev->dev, als_channels,
- sizeof(als_channels), GFP_KERNEL);
- if (!indio_dev->channels) {
- dev_err(&pdev->dev, "failed to duplicate channels\n");
- return -ENOMEM;
- }
-
ret = als_parse_report(pdev, hsdev,
- (struct iio_chan_spec *)indio_dev->channels,
hsdev->usage,
als_state);
if (ret) {
@@ -309,8 +325,14 @@ static int hid_als_probe(struct platform_device *pdev)
return ret;
}
- indio_dev->num_channels =
- ARRAY_SIZE(als_channels);
+ /* Add timestamp channel */
+ als_state->channels[als_state->num_channels] = als_channels[CHANNEL_SCAN_INDEX_TIMESTAMP];
+ als_state->channels[als_state->num_channels].scan_index = als_state->num_channels;
+
+ /* +1 for adding timestamp channel */
+ indio_dev->num_channels = als_state->num_channels + 1;
+
+ indio_dev->channels = als_state->channels;
indio_dev->info = &als_info;
indio_dev->name = name;
indio_dev->modes = INDIO_DIRECT_MODE;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/4] iio: hid-sensor-als: Remove hardcoding of values for enums
From: Srinivas Pandruvada @ 2024-01-09 18:00 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20240109180007.3373784-1-srinivas.pandruvada@linux.intel.com>
Remove hardcoding of values for enum CHANNEL_SCAN_INDEX_INTENSITY and
CHANNEL_SCAN_INDEX_ILLUM.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
v3:
New patch. Added as the next patch removed the hardcoding of enum values
when submitted. To remove unrelated changes, created a patch to just
remove hardcoding of values.
drivers/iio/light/hid-sensor-als.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 72a7c01c97f8..a7bde6b68102 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -14,8 +14,8 @@
#include "../common/hid-sensors/hid-sensor-trigger.h"
enum {
- CHANNEL_SCAN_INDEX_INTENSITY = 0,
- CHANNEL_SCAN_INDEX_ILLUM = 1,
+ CHANNEL_SCAN_INDEX_INTENSITY,
+ CHANNEL_SCAN_INDEX_ILLUM,
CHANNEL_SCAN_INDEX_MAX
};
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/4] iio: hid-sensor-als: Add light color temperature support
From: Srinivas Pandruvada @ 2024-01-09 18:00 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20240109180007.3373784-1-srinivas.pandruvada@linux.intel.com>
From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
On some platforms, ambient color sensors also support light color
temperature. Add support of light color temperature.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
I don't have a system to test this patch.
Hi Basavraj,
Please test.
v3:
Simplilified as no special processing is required in als_parse_report()
v2:
Original patch from Basavaraj Natikar <Basavaraj.Natikar@amd.com> is
modified to prevent failure when the new usage id is not found in the
descriptor.
drivers/iio/light/hid-sensor-als.c | 22 ++++++++++++++++++++++
include/linux/hid-sensor-ids.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index a7bde6b68102..0d54eb59e47d 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -16,6 +16,7 @@
enum {
CHANNEL_SCAN_INDEX_INTENSITY,
CHANNEL_SCAN_INDEX_ILLUM,
+ CHANNEL_SCAN_INDEX_COLOR_TEMP,
CHANNEL_SCAN_INDEX_MAX
};
@@ -43,6 +44,7 @@ struct als_state {
static const u32 als_usage_ids[] = {
HID_USAGE_SENSOR_LIGHT_ILLUM,
HID_USAGE_SENSOR_LIGHT_ILLUM,
+ HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
};
static const u32 als_sensitivity_addresses[] = {
@@ -74,6 +76,16 @@ static const struct iio_chan_spec als_channels[] = {
BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
.scan_index = CHANNEL_SCAN_INDEX_ILLUM,
},
+ {
+ .type = IIO_COLORTEMP,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
+ .scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ },
IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
};
@@ -112,6 +124,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
min = als_state->als[chan->scan_index].logical_minimum;
address = HID_USAGE_SENSOR_LIGHT_ILLUM;
break;
+ case CHANNEL_SCAN_INDEX_COLOR_TEMP:
+ report_id = als_state->als[chan->scan_index].report_id;
+ min = als_state->als[chan->scan_index].logical_minimum;
+ address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
+ break;
default:
report_id = -1;
break;
@@ -235,6 +252,11 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
als_state->scan.illum[scan_index] = sample_data;
ret = 0;
break;
+ case HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE:
+ scan_index = als_state->scan.scan_index[CHANNEL_SCAN_INDEX_COLOR_TEMP];
+ als_state->scan.illum[scan_index] = sample_data;
+ ret = 0;
+ break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
*(s64 *)raw_data);
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 13b1e65fbdcc..8af4fb3e0254 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -21,6 +21,7 @@
#define HID_USAGE_SENSOR_ALS 0x200041
#define HID_USAGE_SENSOR_DATA_LIGHT 0x2004d0
#define HID_USAGE_SENSOR_LIGHT_ILLUM 0x2004d1
+#define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE 0x2004d2
/* PROX (200011) */
#define HID_USAGE_SENSOR_PROX 0x200011
--
2.43.0
^ permalink raw reply related
* [PATCH v3 4/4] iio: hid-sensor-als: Add light chromaticity support
From: Srinivas Pandruvada @ 2024-01-09 18:00 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20240109180007.3373784-1-srinivas.pandruvada@linux.intel.com>
From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
On some platforms, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Add light chromaticity x and y.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
I don't have a system to test this patch.
Hi Basavraj,
Please test.
v3:
Simplilified as no special processing is required in als_parse_report()
v2:
Original patch from Basavaraj Natikar <Basavaraj.Natikar@amd.com> is
modified to prevent failure when the new usage id is not found in the
descriptor.
drivers/iio/light/hid-sensor-als.c | 48 ++++++++++++++++++++++++++++++
include/linux/hid-sensor-ids.h | 3 ++
2 files changed, 51 insertions(+)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 0d54eb59e47d..9c31febc84b8 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -17,6 +17,8 @@ enum {
CHANNEL_SCAN_INDEX_INTENSITY,
CHANNEL_SCAN_INDEX_ILLUM,
CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ CHANNEL_SCAN_INDEX_CHROMATICITY_X,
+ CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
CHANNEL_SCAN_INDEX_MAX
};
@@ -45,6 +47,8 @@ static const u32 als_usage_ids[] = {
HID_USAGE_SENSOR_LIGHT_ILLUM,
HID_USAGE_SENSOR_LIGHT_ILLUM,
HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
+ HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X,
+ HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y,
};
static const u32 als_sensitivity_addresses[] = {
@@ -86,6 +90,30 @@ static const struct iio_chan_spec als_channels[] = {
BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
.scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
},
+ {
+ .type = IIO_CHROMATICITY,
+ .modified = 1,
+ .channel2 = IIO_MOD_X,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
+ .scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X,
+ },
+ {
+ .type = IIO_CHROMATICITY,
+ .modified = 1,
+ .channel2 = IIO_MOD_Y,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
+ .scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
+ },
IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
};
@@ -129,6 +157,16 @@ static int als_read_raw(struct iio_dev *indio_dev,
min = als_state->als[chan->scan_index].logical_minimum;
address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
break;
+ case CHANNEL_SCAN_INDEX_CHROMATICITY_X:
+ report_id = als_state->als[chan->scan_index].report_id;
+ min = als_state->als[chan->scan_index].logical_minimum;
+ address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X;
+ break;
+ case CHANNEL_SCAN_INDEX_CHROMATICITY_Y:
+ report_id = als_state->als[chan->scan_index].report_id;
+ min = als_state->als[chan->scan_index].logical_minimum;
+ address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y;
+ break;
default:
report_id = -1;
break;
@@ -257,6 +295,16 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
als_state->scan.illum[scan_index] = sample_data;
ret = 0;
break;
+ case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X:
+ scan_index = als_state->scan.scan_index[CHANNEL_SCAN_INDEX_CHROMATICITY_X];
+ als_state->scan.illum[scan_index] = sample_data;
+ ret = 0;
+ break;
+ case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y:
+ scan_index = als_state->scan.scan_index[CHANNEL_SCAN_INDEX_CHROMATICITY_Y];
+ als_state->scan.illum[scan_index] = sample_data;
+ ret = 0;
+ break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
*(s64 *)raw_data);
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 8af4fb3e0254..6730ee900ee1 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -22,6 +22,9 @@
#define HID_USAGE_SENSOR_DATA_LIGHT 0x2004d0
#define HID_USAGE_SENSOR_LIGHT_ILLUM 0x2004d1
#define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE 0x2004d2
+#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY 0x2004d3
+#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X 0x2004d4
+#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y 0x2004d5
/* PROX (200011) */
#define HID_USAGE_SENSOR_PROX 0x200011
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] Input: i8042 - add quirk for Lenovo ThinkPad T14 Gen 1
From: Dmitry Torokhov @ 2024-01-09 19:28 UTC (permalink / raw)
To: Jonathan Denose
Cc: linux-input, Jonathan Denose, Hans de Goede, Huacai Chen,
Mattijs Korpershoek, Takashi Iwai, Werner Sembach, linux-kernel
In-Reply-To: <CALNJtpUHHaq6g0wSuyaNBxtOE9kt6vDzdAGGu6j=JJdJmerDWQ@mail.gmail.com>
Hi Jonathan,
On Mon, Nov 27, 2023 at 10:38:57AM -0600, Jonathan Denose wrote:
> Hi Dmitry
>
> On Fri, Nov 24, 2023 at 10:45 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hi Jonathan,
> >
> > On Mon, Sep 25, 2023 at 04:33:20PM -0500, Jonathan Denose wrote:
> > > The ThinkPad T14 Gen 1 touchpad works fine except that clicking
> > > and dragging by tapping the touchpad or depressing the touchpad
> > > do not work. Disabling PNP for controller setting discovery enables
> > > click and drag without negatively impacting other touchpad features.
> >
> > I would like to understand more on how enabling PnP discovery for i8042
> > affects the touchpad. Do you see it using different interrupt or IO
> > ports? What protocol does the touchpad use with/without PnP? If the
> > protocol is the same, do you see difference in the ranges (pressure,
> > etc) reported by the device?
> >
> > Thanks.
> >
> > --
> > Dmitry
>
> Without PnP discovery the touchpad is using the SynPS/2 protocol, with
> PnP discovery, the touchpad is using the rmi4 protocol. Since the
> protocols are different, so are the ranges but let me know if you
> still want to see them.
Thank you for this information. So it is not PnP discovery that appears
harmful in your case, but rather that legacy PS/2 mode appears to be
working better than RMI4 for the device in question.
I will note that the original enablement of RMI4 for T14 was done by
Hans in [1]. Later T14 with AMD were added to the list of devices that
should use RMI4 [2], however this was reverted in [3].
Could you please tell me what exact device you are dealing with? What's
it ACPI ID?
[1] https://lore.kernel.org/all/20201005114919.371592-1-hdegoede@redhat.com/
[2] https://lore.kernel.org/r/20220318113949.32722-1-snafu109@gmail.com
[3] https://lore.kernel.org/r/20220920193936.8709-1-markpearson@lenovo.com
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5 3/5] mfd: cs40l50: Add support for CS40L50 core driver
From: James Ogletree @ 2024-01-09 21:08 UTC (permalink / raw)
To: Charles Keepax
Cc: James Ogletree, Fred Treven, Ben Bright, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Simon Trimmer,
Richard Fitzgerald, Lee Jones, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, James Schulman, David Rhodes,
Sebastian Reichel, Jacky Bai, Jeff LaBundy, Peng Fan,
Weidong Wang, Herve Codina, Arnd Bergmann, Shenghao Ding,
Ryan Lee, Linus Walleij, Maxim Kochetkov, Shuming Fan,
open list:CIRRUS LOGIC HAPTIC DRIVERS,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list,
open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
moderated list:CIRRUS LOGIC AUDIO CODEC DRIVERS
In-Reply-To: <20240105140407.GG14858@ediswmail.ad.cirrus.com>
Hi Charles,
Thank you for your excellent review. Anything not replied to will be
adopted as-is in the next version.
> On Jan 5, 2024, at 8:04 AM, Charles Keepax <ckeepax@opensource.cirrus.com> wrote:
>
> On Thu, Jan 04, 2024 at 10:36:36PM +0000, James Ogletree wrote
>
>> +static int cs40l50_dsp_init(struct cs40l50 *cs40l50)
>> +{
>> + int err;
>> +
>> + cs40l50->dsp.num = 1;
>> + cs40l50->dsp.type = WMFW_HALO;
>> + cs40l50->dsp.dev = cs40l50->dev;
>> + cs40l50->dsp.regmap = cs40l50->regmap;
>> + cs40l50->dsp.base = CS40L50_CORE_BASE;
>> + cs40l50->dsp.base_sysinfo = CS40L50_SYS_INFO_ID;
>> + cs40l50->dsp.mem = cs40l50_dsp_regions;
>> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
>> + cs40l50->dsp.no_core_startstop = true;
>> +
>> + err = cs_dsp_halo_init(&cs40l50->dsp);
>> + if (err)
>> + return err;
>> +
>> + return devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_remove,
>> + &cs40l50->dsp);
>
> Hmm... I notice you use this for both dsp_remove and
> dsp_power_down. Are you sure devm will guarantee those are called
> in the right order? Its not immediately clear to me that would be
> have to be the case.
On my inspection of the devm code, actions are always added to the
tail, and played back from head to tail on driver detach.
>
>> +static int cs40l50_power_up_dsp(struct cs40l50 *cs40l50)
>> +{
>> + int err;
>> +
>> + mutex_lock(&cs40l50->lock);
>> +
>> + if (cs40l50->patch) {
>> + /* Stop core if loading patch file */
>> + err = regmap_multi_reg_write(cs40l50->regmap, cs40l50_stop_core,
>> + ARRAY_SIZE(cs40l50_stop_core));
>> + if (err)
>> + goto err_mutex;
>> + }
>> +
>> + err = cs_dsp_power_up(&cs40l50->dsp, cs40l50->patch, "cs40l50.wmfw",
>> + cs40l50->bin, "cs40l50.bin", "cs40l50");
>> + if (err)
>> + goto err_mutex;
>> +
>> + err = devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_power_down,
>> + &cs40l50->dsp);
>> + if (err)
>> + goto err_mutex;
>> +
>> + if (cs40l50->patch) {
>> + /* Resume core after loading patch file */
>> + err = regmap_write(cs40l50->regmap, CS40L50_CCM_CORE_CONTROL,
>> + CS40L50_CLOCK_ENABLE);
>
> This feels like this needs a comment, why are we skipping the
> normal DSP init and doing it manually (this appears to be the
> same writes start_core would have done)? I assume its something to
> do with what you are really doing is you don't want lock_memory
> to run?
The dsp struct uses cs_dsp_halo_ao_ops, made for self-booting
DSPs, which has none of the ops used in cs_dsp_run(). The
manual stop is because it is self-booting (already running you could
say) but we need to stop the clock to patch the firmware. Please
correct me if that is not right.
>> +static int cs40l50_configure_dsp(struct cs40l50 *cs40l50)
>> +{
>> + u32 nwaves;
>> + int err;
>> +
>> + if (cs40l50->bin) {
>> + /* Log number of effects if wavetable was loaded */
>> + err = regmap_read(cs40l50->regmap, CS40L50_NUM_WAVES, &nwaves);
>> + if (err)
>> + return err;
>> +
>> + dev_info(cs40l50->dev, "Loaded with %u RAM waveforms\n", nwaves);
>
> Kinda nervous about the fact we access all these DSP controls
> directly through address, rather than using the DSP control
> accessors, we have the accessors for a reason. They manage things
> like access permissions etc. and historically, the firmware
> guys have not been able to guarantee these remain in consistent
> locations between firmware versions.
>
> I guess this is so you can access them even in the case of the
> ROM firmware, but you could have a meta-data only firmware file
> that you load in that case to give you the controls. I don't
> feel the need to NAK the driver based on this but please think
> about this very carefully it's a strange way to use the DSP
> controls, and feels likely to cause problems to me. It is also
> quite hostile to fixing it in the future since as you are not
> using the controls no one will be checking that things like the
> access flags in the firmware are set correctly, which is annoying
> if the decision has to be reversed later since there will likely
> be a bunch of broken firmwares already in the field.
Noting here that we discussed this offline. The driver is going to
stay with a static register design for now, but the write sequence
interface is being modified to be control based.
Best,
James
^ permalink raw reply
* Re: [PATCH v4 1/2] i8042: Add forcenorestore quirk to leave controller untouched even on s3
From: Dmitry Torokhov @ 2024-01-09 21:48 UTC (permalink / raw)
To: Werner Sembach; +Cc: hdegoede, linux-input, linux-kernel
In-Reply-To: <20240104183118.779778-2-wse@tuxedocomputers.com>
Hi Werner,
On Thu, Jan 04, 2024 at 07:31:17PM +0100, Werner Sembach wrote:
> On s3 resume the i8042 driver tries to restore the controller to a known
> state by reinitializing things, however this can confuse the controller
> with different effects. Mostly occasionally unresponsive keyboards after
> resume.
>
> These issues do not rise on s0ix resume as here the controller is assumed
> to preserved its state from before suspend.
>
> This patch adds a quirk for devices where the reinitialization on s3 resume
> is not needed and might be harmful as described above. It does this by
> using the s0ix resume code path at selected locations.
>
> This new quirk goes beyond what the preexisting reset=never quirk does,
> which only skips some reinitialization steps.
I think the original change mentioned not only issues on resume, but
also after boot, which this one does not address, at least directly, so
I am not sure if this patch is the proper replacement.
I would also like to understand better what exact step is troublesome,
as I would be surprised if any interaction with the keyboard
controller while suspending causes the issue to manifest. Is it enough,
by chance, to skip restoring MUX mode and reset?
Also, shoudl this system use s2idle by chance?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH AUTOSEL 4.19 4/7] Input: amimouse - convert to platform remove callback returning void
From: Uwe Kleine-König @ 2024-01-09 21:50 UTC (permalink / raw)
To: Pavel Machek
Cc: Sasha Levin, linux-kernel, stable, Dmitry Torokhov, linux-input
In-Reply-To: <ZZ0xt75z/qSf5f8V@duo.ucw.cz>
[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]
On Tue, Jan 09, 2024 at 12:44:55PM +0100, Pavel Machek wrote:
> Hi!
>
> > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >
> > [ Upstream commit 42b8ff47720258d1f6a4412e780a480c139773a0 ]
> >
> > The .remove() callback for a platform driver returns an int which makes
> > many driver authors wrongly assume it's possible to do error handling by
> > returning an error code. However the value returned is ignored (apart
> > from emitting a warning) and this typically results in resource leaks.
> >
> > To improve here there is a quest to make the remove callback return
> > void. In the first step of this quest all drivers are converted to
> > .remove_new(), which already returns void. Eventually after all drivers
> > are converted, .remove_new() will be renamed to .remove().
> >
> > Trivially convert this driver from always returning zero in the remove
> > callback to the void returning variant.
>
> We don't really need this for -stable.
Agreed! These patches shouldn't get backported to stable. Even if they
are a dependency (which isn't the case for this patch AFAICT),
backporting of later patches isn't hard even when dropping these
patches.
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 v5 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
From: James Ogletree @ 2024-01-09 22:03 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: James Ogletree, Fred Treven, Ben Bright, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Simon Trimmer, Charles Keepax,
Richard Fitzgerald, Lee Jones, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, James Schulman, David Rhodes,
Alexandre Belloni, Peng Fan, Jeff LaBundy, Sebastian Reichel,
Jacky Bai, Weidong Wang, Arnd Bergmann, Herve Codina, Shuming Fan,
Shenghao Ding, Ryan Lee, Linus Walleij,
open list:CIRRUS LOGIC HAPTIC DRIVERS,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list,
open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
moderated list:CIRRUS LOGIC AUDIO CODEC DRIVERS
In-Reply-To: <ZZoFUwOEF6NByIp2@google.com>
Hi Dmitry,
Thank you for your excellent review. Just a few questions.
> On Jan 6, 2024, at 7:58 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> On Thu, Jan 04, 2024 at 10:36:37PM +0000, James Ogletree wrote:
>> +
>> + info->add_effect.u.periodic.custom_data = kcalloc(len, sizeof(s16), GFP_KERNEL);
>> + if (!info->add_effect.u.periodic.custom_data)
>> + return -ENOMEM;
>> +
>> + if (copy_from_user(info->add_effect.u.periodic.custom_data,
>> + effect->u.periodic.custom_data, sizeof(s16) * len)) {
>> + info->add_error = -EFAULT;
>> + goto out_free;
>> + }
>> +
>> + queue_work(info->vibe_wq, &info->add_work);
>> + flush_work(&info->add_work);
>
> I do not understand the need of scheduling a work here. You are
> obviously in a sleeping context (otherwise you would not be able to
> execute flush_work()) so you should be able to upload the effect right
> here.
Scheduling work here is to ensure its ordering with “playback" worker
items, which themselves are called in atomic context and so need
deferred work. I think this explains why we need a workqueue as well,
but please correct me.
>
>> +
>> +static int vibra_playback(struct input_dev *dev, int effect_id, int val)
>> +{
>> + struct vibra_info *info = input_get_drvdata(dev);
>> +
>> + if (val > 0) {
>
> value is supposed to signal how many times an effect should be repeated.
> It looks like you are not handling this at all.
For playbacks, we mandate that the input_event value field is set to either 1
or 0 to command either a start playback or stop playback respectively.
Values other than that should be rejected, so in the next version I will fix this
to explicitly check for 1 or 0.
>
>> + info->start_effect = &dev->ff->effects[effect_id];
>> + queue_work(info->vibe_wq, &info->vibe_start_work);
>
> The API allows playback of several effects at once, the way you have it
> done here if multiple requests come at same time only one will be
> handled.
I think I may need some clarification on this point. Why would concurrent
start/stop playback commands get dropped? It seems they would all be
added to the workqueue and executed eventually.
>
>> + } else {
>> + queue_work(info->vibe_wq, &info->vibe_stop_work);
>
> Which effect are you stopping? All of them? You need to stop a
> particular one.
Our implementation of “stop” stops all effects in flight which is intended.
That is probably unusual so I will add a comment here in the next
version.
Best,
James
^ permalink raw reply
* Re: [PATCH v5 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
From: Dmitry Torokhov @ 2024-01-09 22:31 UTC (permalink / raw)
To: James Ogletree
Cc: James Ogletree, Fred Treven, Ben Bright, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Simon Trimmer, Charles Keepax,
Richard Fitzgerald, Lee Jones, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, James Schulman, David Rhodes,
Alexandre Belloni, Peng Fan, Jeff LaBundy, Sebastian Reichel,
Jacky Bai, Weidong Wang, Arnd Bergmann, Herve Codina, Shuming Fan,
Shenghao Ding, Ryan Lee, Linus Walleij,
open list:CIRRUS LOGIC HAPTIC DRIVERS,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list,
open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
moderated list:CIRRUS LOGIC AUDIO CODEC DRIVERS
In-Reply-To: <564A2601-4933-4BD7-B4E6-C973A585CA61@cirrus.com>
On Tue, Jan 09, 2024 at 10:03:02PM +0000, James Ogletree wrote:
> Hi Dmitry,
>
> Thank you for your excellent review. Just a few questions.
>
> > On Jan 6, 2024, at 7:58 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >
> > On Thu, Jan 04, 2024 at 10:36:37PM +0000, James Ogletree wrote:
>
> >> +
> >> + info->add_effect.u.periodic.custom_data = kcalloc(len, sizeof(s16), GFP_KERNEL);
> >> + if (!info->add_effect.u.periodic.custom_data)
> >> + return -ENOMEM;
> >> +
> >> + if (copy_from_user(info->add_effect.u.periodic.custom_data,
> >> + effect->u.periodic.custom_data, sizeof(s16) * len)) {
> >> + info->add_error = -EFAULT;
> >> + goto out_free;
> >> + }
> >> +
> >> + queue_work(info->vibe_wq, &info->add_work);
> >> + flush_work(&info->add_work);
> >
> > I do not understand the need of scheduling a work here. You are
> > obviously in a sleeping context (otherwise you would not be able to
> > execute flush_work()) so you should be able to upload the effect right
> > here.
>
> Scheduling work here is to ensure its ordering with “playback" worker
> items, which themselves are called in atomic context and so need
> deferred work. I think this explains why we need a workqueue as well,
> but please correct me.
>
> >
> >> +
> >> +static int vibra_playback(struct input_dev *dev, int effect_id, int val)
> >> +{
> >> + struct vibra_info *info = input_get_drvdata(dev);
> >> +
> >> + if (val > 0) {
> >
> > value is supposed to signal how many times an effect should be repeated.
> > It looks like you are not handling this at all.
>
> For playbacks, we mandate that the input_event value field is set to either 1
I am sorry, who is "we"?
> or 0 to command either a start playback or stop playback respectively.
> Values other than that should be rejected, so in the next version I will fix this
> to explicitly check for 1 or 0.
No, please implement the API properly.
>
> >
> >> + info->start_effect = &dev->ff->effects[effect_id];
> >> + queue_work(info->vibe_wq, &info->vibe_start_work);
> >
> > The API allows playback of several effects at once, the way you have it
> > done here if multiple requests come at same time only one will be
> > handled.
>
> I think I may need some clarification on this point. Why would concurrent
> start/stop playback commands get dropped? It seems they would all be
> added to the workqueue and executed eventually.
You only have one instance of vibe_start_work, as well as only one
"slot" to hold the effect you want to start. So if you issue 2 request
back to back to play effect 1 and 2 you are likely to end with
info->start_effect == 2 and that is what vibe_start_work handler will
observe, effectively dropping request to play effect 1 on the floor.
>
> >
> >> + } else {
> >> + queue_work(info->vibe_wq, &info->vibe_stop_work);
> >
> > Which effect are you stopping? All of them? You need to stop a
> > particular one.
>
> Our implementation of “stop” stops all effects in flight which is intended.
> That is probably unusual so I will add a comment here in the next
> version.
Again, please implement the driver properly, not define your own
carveouts for the expected behavior.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: Remove SET_POWER SLEEP on system suspend
From: Kai-Heng Feng @ 2024-01-10 7:31 UTC (permalink / raw)
To: Doug Anderson
Cc: jikos, benjamin.tissoires, Maxime Ripard, Thomas Weißschuh,
Johan Hovold, Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <CAD=FV=VmGNB5dP5WO7=txNDScNfhCDEsfFFivXqz+PH6rt=x8g@mail.gmail.com>
On Tue, Jan 9, 2024 at 5:25 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Jan 2, 2024 at 4:34 PM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
> >
> > There's a Cirque touchpad that wakes system up without anything touched
> > the touchpad. The input report is empty when this happens.
> > The reason is stated in HID over I2C spec, 7.2.8.2:
> > "If the DEVICE wishes to wake the HOST from its low power state, it can
> > issue a wake by asserting the interrupt."
> >
> > This is fine if OS can put system back to suspend by identifying input
> > wakeup count stays the same on resume, like Chrome OS Dark Resume [0].
> > But for regular distro such policy is lacking.
> >
> > According to commit d9f448e3d71f ("HID: i2c-hid: set power sleep before
> > shutdown"), SLEEP is required for shutdown, in addition to that, commit
> > 67b18dfb8cfc ("HID: i2c-hid: Remove runtime power management") didn't
> > notice any power comsumption reduction from SET_POWER SLEEP, so also
> > remove that to avoid the device asserting the interrupt.
> >
> > [0] https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/power_manager/docs/dark_resume.md
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > drivers/hid/i2c-hid/i2c-hid-core.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> > index 2735cd585af0..dd513dc75cb9 100644
> > --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> > +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> > @@ -957,9 +957,6 @@ static int i2c_hid_core_suspend(struct i2c_hid *ihid, bool force_poweroff)
> > if (ret < 0)
> > return ret;
> >
> > - /* Save some power */
> > - i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
>
> IMO this is not a good idea to do universally. Specifically:
>
> 1. There are many vendors of i2c-hid devices and many different
> devices per vendor. Even if your device doesn't save power in "sleep"
> mode, that doesn't mean it's not important for some other devices.
Thanks for the response.
>
> 2. There are some boards where an i2c-hid device is powered by an
> "always-on" regulator. On these devices it seems like a bad idea not
> to be able to put the i2c-hid device into sleep mode.
>
So if SLEEP for system wide suspend is crucial for some i2c-hid
devices, it should be kept.
>
> I'd also note that I'm really not sure what ChromeOS dark resume has
> to do with anything here. Dark resume is used for certain types of
> events that wakeup the system where we can identify that the event
> shouldn't turn the screen on, then we do some processing, then we go
> back to sleep. I'm nearly certain that a trackpad / touchscreen wakeup
> event would never qualify for "dark resume". If we see a
> trackpad/touchscreen event then we'll wakeup the system. If the system
> is in a state where trackpad/touchscreen events shouldn't wake us up
> then we disable those wakeups before going to suspend...
Doesn't Dark Resume use wakeup count to decide whether the system
should wake up or go back to suspend?
For this case the input report is empty, hence wakeup count remains
the same after the wakeup. I assumed Dark Resume will check the wakeup
count and decide to put the system back to suspend.
>
> It seems to me like the board you're testing on has some strange bug
> and that bug should be fixed, or (in the worst case) you should send a
> patch to detect this broken touchpad and disable wakeup for it.
It's desired to keep the wakeup capability, disabling wakeup isn't ideal here.
I'll write a patch to use touchpad specific quirk instead of applying
the change universally.
Kai-Heng
>
>
> -Doug
^ permalink raw reply
* Re: [PATCH v4 1/2] i8042: Add forcenorestore quirk to leave controller untouched even on s3
From: Werner Sembach @ 2024-01-10 13:51 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: hdegoede, linux-input, linux-kernel
In-Reply-To: <ZZ2_KMGdokHNWcNL@google.com>
Am 09.01.24 um 22:48 schrieb Dmitry Torokhov:
> Hi Werner,
>
> On Thu, Jan 04, 2024 at 07:31:17PM +0100, Werner Sembach wrote:
>> On s3 resume the i8042 driver tries to restore the controller to a known
>> state by reinitializing things, however this can confuse the controller
>> with different effects. Mostly occasionally unresponsive keyboards after
>> resume.
>>
>> These issues do not rise on s0ix resume as here the controller is assumed
>> to preserved its state from before suspend.
>>
>> This patch adds a quirk for devices where the reinitialization on s3 resume
>> is not needed and might be harmful as described above. It does this by
>> using the s0ix resume code path at selected locations.
>>
>> This new quirk goes beyond what the preexisting reset=never quirk does,
>> which only skips some reinitialization steps.
> I think the original change mentioned not only issues on resume, but
> also after boot, which this one does not address, at least directly, so
> I am not sure if this patch is the proper replacement.
The original change introduced issues after boot and fixed issues after resume.
The new quirk fixes the issues after resume without introducing issues after boot.
The issues after boot where only affecting the NHxxRZQ and the N1xxCU iirc.
>
> I would also like to understand better what exact step is troublesome,
> as I would be surprised if any interaction with the keyboard
> controller while suspending causes the issue to manifest. Is it enough,
> by chance, to skip restoring MUX mode and reset?
SERIO_QUIRK_NOMUX and SERIO_QUIRK_RESET_ALWAYS are required in the old fix for
the resume issues to go away (I don't know if SERIO_QUIRK_NOLOOP and
SERIO_QUIRK_NOPNP are required, I threw them in just in case because we were
running them already on the device, so they where somewhat "field proven" to not
break things).
However while SERIO_QUIRK_NOMUX and SERIO_QUIRK_RESET_ALWAYS for themself did
not individually introduce the boot problem, in combination they did.
That was all I was able to find out back when I tested the old quirks.
>
> Also, shoudl this system use s2idle by chance?
N1xxCU is s3 only (intel 10th gen clevo)
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH v5 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
From: James Ogletree @ 2024-01-10 14:36 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: James Ogletree, Fred Treven, Ben Bright, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Simon Trimmer, Charles Keepax,
Richard Fitzgerald, Lee Jones, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, James Schulman, David Rhodes,
Alexandre Belloni, Peng Fan, Jeff LaBundy, Sebastian Reichel,
Jacky Bai, Weidong Wang, Arnd Bergmann, Herve Codina, Shuming Fan,
Shenghao Ding, Ryan Lee, Linus Walleij,
open list:CIRRUS LOGIC HAPTIC DRIVERS,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list,
open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
moderated list:CIRRUS LOGIC AUDIO CODEC DRIVERS
In-Reply-To: <ZZ3JNuLp-ZfGOQRF@google.com>
> On Jan 9, 2024, at 4:31 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> On Tue, Jan 09, 2024 at 10:03:02PM +0000, James Ogletree wrote:
>> Hi Dmitry,
>>
>> Thank you for your excellent review. Just a few questions.
>>
>>> On Jan 6, 2024, at 7:58 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>>>
>>> On Thu, Jan 04, 2024 at 10:36:37PM +0000, James Ogletree wrote:
>>>> +
>>>> + info->add_effect.u.periodic.custom_data = kcalloc(len, sizeof(s16), GFP_KERNEL);
>>>> + if (!info->add_effect.u.periodic.custom_data)
>>>> + return -ENOMEM;
>>>> +
>>>> + if (copy_from_user(info->add_effect.u.periodic.custom_data,
>>>> + effect->u.periodic.custom_data, sizeof(s16) * len)) {
>>>> + info->add_error = -EFAULT;
>>>> + goto out_free;
>>>> + }
>>>> +
>>>> + queue_work(info->vibe_wq, &info->add_work);
>>>> + flush_work(&info->add_work);
>>>
>>> I do not understand the need of scheduling a work here. You are
>>> obviously in a sleeping context (otherwise you would not be able to
>>> execute flush_work()) so you should be able to upload the effect right
>>> here.
>>
>> Scheduling work here is to ensure its ordering with “playback" worker
>> items, which themselves are called in atomic context and so need
>> deferred work. I think this explains why we need a workqueue as well,
>> but please correct me.
>>
>>>
>>>> +
>>>> +static int vibra_playback(struct input_dev *dev, int effect_id, int val)
>>>> +{
>>>> + struct vibra_info *info = input_get_drvdata(dev);
>>>> +
>>>> + if (val > 0) {
>>>
>>> value is supposed to signal how many times an effect should be repeated.
>>> It looks like you are not handling this at all.
>>
>> For playbacks, we mandate that the input_event value field is set to either 1
>
> I am sorry, who is "we"?
Just a royal “I”. Apologies, no claim to authority intended here. :)
>
>> or 0 to command either a start playback or stop playback respectively.
>> Values other than that should be rejected, so in the next version I will fix this
>> to explicitly check for 1 or 0.
>
> No, please implement the API properly.
Ack.
>
>>
>>>
>>>> + info->start_effect = &dev->ff->effects[effect_id];
>>>> + queue_work(info->vibe_wq, &info->vibe_start_work);
>>>
>>> The API allows playback of several effects at once, the way you have it
>>> done here if multiple requests come at same time only one will be
>>> handled.
>>
>> I think I may need some clarification on this point. Why would concurrent
>> start/stop playback commands get dropped? It seems they would all be
>> added to the workqueue and executed eventually.
>
> You only have one instance of vibe_start_work, as well as only one
> "slot" to hold the effect you want to start. So if you issue 2 request
> back to back to play effect 1 and 2 you are likely to end with
> info->start_effect == 2 and that is what vibe_start_work handler will
> observe, effectively dropping request to play effect 1 on the floor.
Understood, ack.
>
>>
>>>
>>>> + } else {
>>>> + queue_work(info->vibe_wq, &info->vibe_stop_work);
>>>
>>> Which effect are you stopping? All of them? You need to stop a
>>> particular one.
>>
>> Our implementation of “stop” stops all effects in flight which is intended.
>> That is probably unusual so I will add a comment here in the next
>> version.
>
> Again, please implement the driver properly, not define your own
> carveouts for the expected behavior.
Ack, and a clarification question: the device is not actually able to
play multiple effects at once. In that case, does stopping a specific
effect entail just cancelling an effect in the queue?
Best,
James
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: Remove SET_POWER SLEEP on system suspend
From: Doug Anderson @ 2024-01-10 18:34 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: jikos, benjamin.tissoires, Maxime Ripard, Thomas Weißschuh,
Johan Hovold, Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <CAAd53p4gPkbDyNLiYGtcvqWwEgBqVhri+qgh+=Ha0xsVfYy92g@mail.gmail.com>
Hi,
On Tue, Jan 9, 2024 at 11:31 PM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> > I'd also note that I'm really not sure what ChromeOS dark resume has
> > to do with anything here. Dark resume is used for certain types of
> > events that wakeup the system where we can identify that the event
> > shouldn't turn the screen on, then we do some processing, then we go
> > back to sleep. I'm nearly certain that a trackpad / touchscreen wakeup
> > event would never qualify for "dark resume". If we see a
> > trackpad/touchscreen event then we'll wakeup the system. If the system
> > is in a state where trackpad/touchscreen events shouldn't wake us up
> > then we disable those wakeups before going to suspend...
>
> Doesn't Dark Resume use wakeup count to decide whether the system
> should wake up or go back to suspend?
> For this case the input report is empty, hence wakeup count remains
> the same after the wakeup. I assumed Dark Resume will check the wakeup
> count and decide to put the system back to suspend.
Ah, I understand now. So you're saying that the issue wouldn't be so
bad (or maybe we wouldn't notice it) on systems with dark resume.
However, even with dark resume we're not in a super great shape. Doing
a dark resume isn't exactly a lightweight operation, since it can take
a bit of time to resume the system, realize that there were no wakeup
events, and then go back to sleep. I'm not a total expert on dark
resume, but I believe that even with dark resume, there may also be
artifacts that a user might notice (like perhaps USB devices powering
up or perhaps the suspend LED on the system showing that we're not in
suspend anymore).
> > It seems to me like the board you're testing on has some strange bug
> > and that bug should be fixed, or (in the worst case) you should send a
> > patch to detect this broken touchpad and disable wakeup for it.
>
> It's desired to keep the wakeup capability, disabling wakeup isn't ideal here.
> I'll write a patch to use touchpad specific quirk instead of applying
> the change universally.
Thanks! I'd also be curious if this is a problem for everyone with the
Cirque touchpad or if it's board-specific. I could imagine the
behavior you describe as coming about due to a missing or
misconfigured pull resistor on the IRQ line. ...or perhaps a pull
resistor pulling up to the wrong voltage rail...
-Doug
^ permalink raw reply
* Re: [PATCH] Input: i8042 - add quirk for Lenovo ThinkPad T14 Gen 1
From: Jonathan Denose @ 2024-01-10 23:42 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Jonathan Denose, Hans de Goede, Huacai Chen,
Mattijs Korpershoek, Takashi Iwai, Werner Sembach, linux-kernel
In-Reply-To: <ZZ2eduF_h7lcBrSL@google.com>
Dmitry,
Sorry I forgot to reply all, so I'm resending my other email.
On Tue, Jan 9, 2024 at 1:28 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Jonathan,
>
> On Mon, Nov 27, 2023 at 10:38:57AM -0600, Jonathan Denose wrote:
> > Hi Dmitry
> >
> > On Fri, Nov 24, 2023 at 10:45 PM Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > >
> > > Hi Jonathan,
> > >
> > > On Mon, Sep 25, 2023 at 04:33:20PM -0500, Jonathan Denose wrote:
> > > > The ThinkPad T14 Gen 1 touchpad works fine except that clicking
> > > > and dragging by tapping the touchpad or depressing the touchpad
> > > > do not work. Disabling PNP for controller setting discovery enables
> > > > click and drag without negatively impacting other touchpad features.
> > >
> > > I would like to understand more on how enabling PnP discovery for i8042
> > > affects the touchpad. Do you see it using different interrupt or IO
> > > ports? What protocol does the touchpad use with/without PnP? If the
> > > protocol is the same, do you see difference in the ranges (pressure,
> > > etc) reported by the device?
> > >
> > > Thanks.
> > >
> > > --
> > > Dmitry
> >
> > Without PnP discovery the touchpad is using the SynPS/2 protocol, with
> > PnP discovery, the touchpad is using the rmi4 protocol. Since the
> > protocols are different, so are the ranges but let me know if you
> > still want to see them.
>
> Thank you for this information. So it is not PnP discovery that appears
> harmful in your case, but rather that legacy PS/2 mode appears to be
> working better than RMI4 for the device in question.
>
> I will note that the original enablement of RMI4 for T14 was done by
> Hans in [1]. Later T14 with AMD were added to the list of devices that
> should use RMI4 [2], however this was reverted in [3].
>
> Could you please tell me what exact device you are dealing with? What's
> it ACPI ID?
>
> [1] https://lore.kernel.org/all/20201005114919.371592-1-hdegoede@redhat.com/
> [2] https://lore.kernel.org/r/20220318113949.32722-1-snafu109@gmail.com
> [3] https://lore.kernel.org/r/20220920193936.8709-1-markpearson@lenovo.com
>
> Thanks.
>
> --
> Dmitry
Thanks for your reply!
I'm not 100% sure which of these is the ACPI ID, but from `udevadm
info -e` there's:
N: Name="Synaptics TM3471-020"
P: Phys=rmi4-00/input0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox