Move no_feedback DMI check to the companion driver

drivers/misc/ipts/companion.c

@@ -209,3 +209,22 @@ int ipts_request_firmware_config(struct ipts_info *ipts,
 	return ret;
 
 }
+
+unsigned int ipts_get_quirks(void)
+{
+	unsigned int ret;
+
+	// Make sure that access to the companion is synchronized
+	mutex_lock(&ipts_companion_lock);
+
+	// If the companion is ignored, or doesn't exist, assume that
+	// the device doesn't have any quirks
+	if (ipts_modparams.ignore_companion || ipts_companion == NULL)
+		ret = IPTS_QUIRK_NONE;
+	else
+		ret = ipts_companion->get_quirks(ipts_companion);
+
+	mutex_unlock(&ipts_companion_lock);
+
+	return ret;
+}

drivers/misc/ipts/companion.h

@@ -15,6 +15,7 @@
 #include "ipts.h"
 
 bool ipts_companion_available(void);
+unsigned int ipts_get_quirks(void);
 
 int ipts_request_firmware(const struct firmware **fw, const char *name,
 		struct device *device);

drivers/misc/ipts/companion/ipts-surface.c

@@ -26,19 +26,64 @@
 	MODULE_FIRMWARE("intel/ipts/" X "/vendor_desc.bin");		\
 	MODULE_FIRMWARE("intel/ipts/" X "/vendor_kernel.bin")
 
+struct ipts_surface_data {
+	const char *hid;
+	unsigned int quirks;
+};
+
+// Surface Book 1 / Surface Studio
+static const struct ipts_surface_data ipts_surface_mshw0076 = {
+	.hid = "MSHW0076",
+	.quirks = IPTS_QUIRK_NO_FEEDBACK,
+};
+
+// Surface Pro 4
+static const struct ipts_surface_data ipts_surface_mshw0078 = {
+	.hid = "MSHW0078",
+	.quirks = IPTS_QUIRK_NO_FEEDBACK,
+};
+
+// Surface Laptop 1 / 2
+static const struct ipts_surface_data ipts_surface_mshw0079 = {
+	.hid = "MSHW0079",
+	.quirks = IPTS_QUIRK_NONE,
+};
+
+// Surface Pro 5 / 6
+static const struct ipts_surface_data ipts_surface_mshw0101 = {
+	.hid = "MSHW0101",
+	.quirks = IPTS_QUIRK_NONE,
+};
+
+// Surface Book 2 15"
+static const struct ipts_surface_data ipts_surface_mshw0102 = {
+	.hid = "MSHW0102",
+	.quirks = IPTS_QUIRK_NONE,
+};
+
+// Unknown, but firmware exists
+static const struct ipts_surface_data ipts_surface_mshw0103 = {
+	.hid = "MSHW0103",
+	.quirks = IPTS_QUIRK_NONE,
+};
+
+// Surface Book 2 13"
+static const struct ipts_surface_data ipts_surface_mshw0137 = {
+	.hid = "MSHW0137",
+	.quirks = IPTS_QUIRK_NONE,
+};
+
+/*
+ * Checkpatch complains about the following lines because it sees them as
+ * header files mixed with .c files. However, forward declaration is perfectly
+ * fine in C, and this allows us to seperate the companion data from the
+ * functions for the companion.
+ */
 int ipts_surface_request_firmware(struct ipts_companion *companion,
 		const struct firmware **fw, const char *name,
-		struct device *device)
-{
-	char fw_path[MAX_IOCL_FILE_PATH_LEN];
+		struct device *device);
 
-	if (companion == NULL || companion->data == NULL)
-		return -ENOENT;
-
-	snprintf(fw_path, MAX_IOCL_FILE_PATH_LEN, IPTS_SURFACE_FW_PATH_FMT,
-		(const char *)companion->data, name);
-	return request_firmware(fw, fw_path, device);
-}
+unsigned int ipts_surface_get_quirks(struct ipts_companion *companion);
 
 static struct ipts_bin_fw_info ipts_surface_vendor_kernel = {
 	.fw_name = "vendor_kernel.bin",
@@ -74,20 +119,53 @@ static struct ipts_bin_fw_info *ipts_surface_fw_config[] = {
 static struct ipts_companion ipts_surface_companion = {
 	.firmware_request = &ipts_surface_request_firmware,
 	.firmware_config = ipts_surface_fw_config,
+	.get_quirks = &ipts_surface_get_quirks,
 	.name = "ipts_surface",
 };
 
+int ipts_surface_request_firmware(struct ipts_companion *companion,
+		const struct firmware **fw, const char *name,
+		struct device *device)
+{
+	char fw_path[MAX_IOCL_FILE_PATH_LEN];
+	struct ipts_surface_data *data;
+
+	if (companion == NULL || companion->data == NULL)
+		return -ENOENT;
+
+	data = (struct ipts_surface_data *)companion->data;
+
+	snprintf(fw_path, MAX_IOCL_FILE_PATH_LEN, IPTS_SURFACE_FW_PATH_FMT,
+		data->hid, name);
+	return request_firmware(fw, fw_path, device);
+}
+
+unsigned int ipts_surface_get_quirks(struct ipts_companion *companion)
+{
+	struct ipts_surface_data *data;
+
+	// In case something went wrong, assume that the
+	// device doesn't have any quirks
+	if (companion == NULL || companion->data == NULL)
+		return IPTS_QUIRK_NONE;
+
+	data = (struct ipts_surface_data *)companion->data;
+
+	return data->quirks;
+}
+
 static int ipts_surface_probe(struct platform_device *pdev)
 {
 	int r;
-	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+	const struct ipts_surface_data *data =
+		acpi_device_get_match_data(&pdev->dev);
 
-	if (!adev) {
+	if (!data) {
 		dev_err(&pdev->dev, "Unable to find ACPI info for device\n");
 		return -ENODEV;
 	}
 
-	ipts_surface_companion.data = (void *)acpi_device_hid(adev);
+	ipts_surface_companion.data = (void *)data;
 
 	r = ipts_add_companion(&ipts_surface_companion);
 	if (r) {
@@ -111,13 +189,13 @@ static int ipts_surface_remove(struct platform_device *pdev)
 }
 
 static const struct acpi_device_id ipts_surface_acpi_match[] = {
-	{ "MSHW0076", 0 },	// Surface Book 1 / Surface Studio
-	{ "MSHW0078", 0 },	// Surface Pro 4
-	{ "MSHW0079", 0 },	// Surface Laptop 1 / 2
-	{ "MSHW0101", 0 },	// Surface Book 2 15"
-	{ "MSHW0102", 0 },	// Surface Pro 2017 / 6
-	{ "MSHW0103", 0 },	// unknown, but firmware exists
-	{ "MSHW0137", 0 },	// Surface Book 2
+	{ "MSHW0076", (unsigned long)&ipts_surface_mshw0076 },
+	{ "MSHW0078", (unsigned long)&ipts_surface_mshw0078 },
+	{ "MSHW0079", (unsigned long)&ipts_surface_mshw0079 },
+	{ "MSHW0101", (unsigned long)&ipts_surface_mshw0101 },
+	{ "MSHW0102", (unsigned long)&ipts_surface_mshw0102 },
+	{ "MSHW0103", (unsigned long)&ipts_surface_mshw0103 },
+	{ "MSHW0137", (unsigned long)&ipts_surface_mshw0137 },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, ipts_surface_acpi_match);
@@ -142,4 +220,5 @@ IPTS_SURFACE_FIRMWARE("MSHW0079");
 IPTS_SURFACE_FIRMWARE("MSHW0101");
 IPTS_SURFACE_FIRMWARE("MSHW0102");
 IPTS_SURFACE_FIRMWARE("MSHW0103");
+
 IPTS_SURFACE_FIRMWARE("MSHW0137");

drivers/misc/ipts/hid.c

@@ -9,6 +9,7 @@
 #include <linux/dmi.h>
 #include <linux/firmware.h>
 #include <linux/hid.h>
+#include <linux/ipts.h>
 #include <linux/module.h>
 #include <linux/vmalloc.h>
 
@@ -47,24 +48,6 @@ struct kernel_output_payload_error {
 	char string[128];
 };
 
-static const struct dmi_system_id no_feedback_dmi_table[] = {
-	{
-		.matches = {
-			DMI_EXACT_MATCH(DMI_SYS_VENDOR,
-					"Microsoft Corporation"),
-			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book"),
-		},
-	},
-	{
-		.matches = {
-			DMI_EXACT_MATCH(DMI_SYS_VENDOR,
-					"Microsoft Corporation"),
-			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 4"),
-		},
-	},
-	{ }
-};
-
 static int ipts_hid_get_descriptor(struct ipts_info *ipts,
 		u8 **desc, int *size)
 {
@@ -448,6 +431,10 @@ static int handle_outputs(struct ipts_info *ipts, int parallel_idx)
 	 * The most desirable fix could be done by raising IPTS GuC priority.
 	 * Until we find a better solution, use this workaround.
 	 *
+	 * The decision which devices have no_feedback enabled by default is
+	 * made by the companion driver. If no companion driver was loaded,
+	 * no_feedback is disabled and the default behaviour is used.
+	 *
 	 * Link to the comment where sebanc found this workaround:
 	 * https://github.com/jakeday/linux-surface/issues/374#issuecomment-508234110
 	 * (Touch and pen issue persists · Issue #374 · jakeday/linux-surface)
@@ -459,7 +446,7 @@ static int handle_outputs(struct ipts_info *ipts, int parallel_idx)
 	if (fb_buf) {
 		// A negative value means "decide by dmi table"
 		if (ipts_modparams.no_feedback < 0) {
-			if (dmi_check_system(no_feedback_dmi_table))
+			if (ipts_get_quirks() & IPTS_QUIRK_NO_FEEDBACK)
 				ipts_modparams.no_feedback = true;
 			else
 				ipts_modparams.no_feedback = false;

include/linux/ipts-companion.h

@@ -14,6 +14,7 @@
 #include <linux/ipts-binary.h>
 
 struct ipts_companion {
+	unsigned int (*get_quirks)(struct ipts_companion *companion);
 	int (*firmware_request)(struct ipts_companion *companion,
 		const struct firmware **fw,
 		const char *name, struct device *device);

include/linux/ipts.h

@@ -9,7 +9,12 @@
 #ifndef IPTS_H
 #define IPTS_H
 
+#include <linux/bits.h>
+
 #define MAX_IOCL_FILE_NAME_LEN 80
 #define MAX_IOCL_FILE_PATH_LEN 256
 
+#define IPTS_QUIRK_NONE        0
+#define IPTS_QUIRK_NO_FEEDBACK BIT(0)
+
 #endif // IPTS_H