Add custom wrapper over kthread, fixes MEI commands during kthread_stop

Thanks to @qzed for figuring out that trying to call mei_cldev_send inside
of a thread after kthread_stop has been called will always result in -EINTR
being returned.

src/Makefile

@@ -11,3 +11,4 @@ ipts-objs += main.o
 ipts-objs += mei.o
 ipts-objs += receiver.o
 ipts-objs += resources.o
+ipts-objs += thread.o

src/context.h

@@ -20,6 +20,7 @@
 #include "mei.h"
 #include "resources.h"
 #include "spec-device.h"
+#include "thread.h"
 
 struct ipts_context {
 	struct device *dev;
@@ -41,7 +42,7 @@ struct ipts_context {
 	struct ipts_device_info info;
 	struct ipts_resources resources;
 
-	struct task_struct *receiver_loop;
+	struct ipts_thread receiver_loop;
 };
 
 #endif /* IPTS_CONTEXT_H */

src/receiver.c

@@ -19,6 +19,7 @@
 #include "hid.h"
 #include "resources.h"
 #include "spec-device.h"
+#include "thread.h"
 
 static void ipts_receiver_next_doorbell(struct ipts_context *ipts)
 {
@@ -46,20 +47,25 @@ static void ipts_receiver_backoff(time64_t last, u32 n)
 		msleep(200);
 }
 
-static int ipts_receiver_event_loop(void *data)
+static int ipts_receiver_event_loop(struct ipts_thread *thread)
 {
 	int ret = 0;
 	u32 buffer = 0;
 
-	struct ipts_context *ipts = data;
+	struct ipts_context *ipts = NULL;
 	time64_t last = ktime_get_seconds();
 
+	if (!thread)
+		return -EFAULT;
+
+	ipts = thread->data;
+
 	if (!ipts)
 		return -EFAULT;
 
 	dev_info(ipts->dev, "IPTS running in event mode\n");
 
-	while (!kthread_should_stop()) {
+	while (!ipts_thread_should_stop(thread)) {
 		for (int i = 0; i < IPTS_BUFFERS; i++) {
 			ret = ipts_control_wait_data(ipts, false);
 			if (ret == -EAGAIN)
@@ -120,24 +126,29 @@ static int ipts_receiver_event_loop(void *data)
 	return 0;
 }
 
-static int ipts_receiver_doorbell_loop(void *data)
+static int ipts_receiver_doorbell_loop(struct ipts_thread *thread)
 {
 	int ret = 0;
 	u32 buffer = 0;
 
 	u32 doorbell = 0;
 	u32 lastdb = 0;
 
-	struct ipts_context *ipts = data;
+	struct ipts_context *ipts = NULL;
 	time64_t last = ktime_get_seconds();
 
+	if (!thread)
+		return -EFAULT;
+
+	ipts = thread->data;
+
 	if (!ipts)
 		return -EFAULT;
 
 	dev_info(ipts->dev, "IPTS running in doorbell mode\n");
 
 	while (true) {
-		if (kthread_should_stop()) {
+		if (ipts_thread_should_stop(thread)) {
 			ret = ipts_control_request_flush(ipts);
 			if (ret) {
 				dev_err(ipts->dev, "Failed to request flush: %d\n", ret);
@@ -167,7 +178,7 @@ static int ipts_receiver_doorbell_loop(void *data)
 			lastdb++;
 		}
 
-		if (kthread_should_stop())
+		if (ipts_thread_should_stop(thread))
 			break;
 
 		ipts_receiver_backoff(last, 5);
@@ -198,21 +209,24 @@ static int ipts_receiver_doorbell_loop(void *data)
 
 int ipts_receiver_start(struct ipts_context *ipts)
 {
+	int ret = 0;
+
 	if (!ipts)
 		return -EFAULT;
 
-	if (ipts->mode == IPTS_MODE_EVENT)
-		ipts->receiver_loop = kthread_run(ipts_receiver_event_loop, ipts, "ipts_event");
-	else if (ipts->mode == IPTS_MODE_DOORBELL)
-		ipts->receiver_loop = kthread_run(ipts_receiver_doorbell_loop, ipts, "ipts_db");
-	else
-		return -EINVAL;
-
-	if (IS_ERR(ipts->receiver_loop)) {
-		int err = PTR_ERR(ipts->receiver_loop);
+	if (ipts->mode == IPTS_MODE_EVENT) {
+		ret = ipts_thread_start(&ipts->receiver_loop, ipts_receiver_event_loop, ipts,
+					"ipts_event");
+	} else if (ipts->mode == IPTS_MODE_DOORBELL) {
+		ret = ipts_thread_start(&ipts->receiver_loop, ipts_receiver_doorbell_loop, ipts,
+					"ipts_doorbell");
+	} else {
+		ret = -EINVAL;
+	}
 
-		dev_err(ipts->dev, "Failed to start receiver loop: %d\n", err);
-		return err;
+	if (ret) {
+		dev_err(ipts->dev, "Failed to start receiver loop: %d\n", ret);
+		return ret;
 	}
 
 	return 0;
@@ -225,12 +239,7 @@ int ipts_receiver_stop(struct ipts_context *ipts)
 	if (!ipts)
 		return -EFAULT;
 
-	if (!ipts->receiver_loop)
-		return 0;
-
-	ret = kthread_stop(ipts->receiver_loop);
-	ipts->receiver_loop = NULL;
-
+	ret = ipts_thread_stop(&ipts->receiver_loop);
 	if (ret) {
 		dev_err(ipts->dev, "Failed to stop receiver loop: %d\n", ret);
 		return ret;

src/thread.c

@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2016 Intel Corporation
+ * Copyright (c) 2020-2022 Dorian Stoll
+ *
+ * Linux driver for Intel Precise Touch & Stylus
+ */
+
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/kthread.h>
+#include <linux/mutex.h>
+
+#include "thread.h"
+
+bool ipts_thread_should_stop(struct ipts_thread *thread)
+{
+	bool ret = false;
+
+	if (!thread)
+		return false;
+
+	mutex_lock(&thread->lock);
+	ret = thread->should_stop;
+	mutex_unlock(&thread->lock);
+
+	return ret;
+}
+
+static int ipts_thread_runner(void *data)
+{
+	int ret = 0;
+	struct ipts_thread *thread = data;
+
+	if (!thread)
+		return -EFAULT;
+
+	if (!thread->threadfn)
+		return -EFAULT;
+
+	ret = thread->threadfn(thread);
+	complete_all(&thread->done);
+
+	return ret;
+}
+
+int ipts_thread_start(struct ipts_thread *thread, int (*threadfn)(struct ipts_thread *thread),
+		      void *data, const char *name)
+{
+	if (!thread)
+		return -EFAULT;
+
+	if (!threadfn)
+		return -EFAULT;
+
+	mutex_init(&thread->lock);
+	init_completion(&thread->done);
+
+	thread->data = data;
+	thread->should_stop = false;
+	thread->threadfn = threadfn;
+
+	thread->thread = kthread_run(ipts_thread_runner, thread, name);
+	return PTR_ERR_OR_ZERO(thread->thread);
+}
+
+int ipts_thread_stop(struct ipts_thread *thread)
+{
+	int ret = 0;
+
+	if (!thread)
+		return -EFAULT;
+
+	if (!thread->thread)
+		return 0;
+
+	mutex_lock(&thread->lock);
+	thread->should_stop = true;
+	mutex_unlock(&thread->lock);
+
+	wait_for_completion(&thread->done);
+	ret = kthread_stop(thread->thread);
+
+	thread->thread = NULL;
+	thread->data = NULL;
+	thread->threadfn = NULL;
+
+	return ret;
+}

src/thread.h

@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2016 Intel Corporation
+ * Copyright (c) 2020-2022 Dorian Stoll
+ *
+ * Linux driver for Intel Precise Touch & Stylus
+ */
+
+#ifndef IPTS_THREAD_H
+#define IPTS_THREAD_H
+
+#include <linux/completion.h>
+#include <linux/mutex.h>
+#include <linux/sched.h>
+
+/*
+ * This wrapper over kthread is neccessary, because calling kthread_stop makes it impossible
+ * to issue MEI commands from that thread while it shuts itself down. By using a custom
+ * boolean variable and a completion object, we can call kthread_stop only when the thread
+ * already finished all of its work and has returned.
+*/
+struct ipts_thread {
+	struct task_struct *thread;
+
+	bool should_stop;
+
+	struct mutex lock;
+	struct completion done;
+
+	void *data;
+	int (*threadfn)(struct ipts_thread *thread);
+};
+
+/*
+ * ipts_thread_should_stop() - Returns true if the thread is asked to terminate.
+ * @thread: The current thread.
+ *
+ * Returns: true if the thread should stop, false if not.
+ */
+bool ipts_thread_should_stop(struct ipts_thread *thread);
+
+/*
+ * ipts_thread_start() - Starts an IPTS thread.
+ * @thread: The thread to initialize and start.
+ * @threadfn: The function to execute.
+ * @data: An argument that will be passed to threadfn.
+ * @name: The name of the new thread.
+ *
+ * Returns: 0 on success, <0 on error.
+ */
+int ipts_thread_start(struct ipts_thread *thread, int (*threadfn)(struct ipts_thread *thread),
+		      void *data, const char name[]);
+
+/*
+ * ipts_thread_stop() - Asks the thread to terminate and waits until it has finished.
+ * @thread: The thread that should stop.
+ *
+ * Returns: The return value of the thread function.
+ */
+int ipts_thread_stop(struct ipts_thread *thread);
+
+#endif /* IPTS_THREAD_H */