[PATCH char-misc-next 0/4] mei: csc: add and enable polling option

[PATCH char-misc-next 0/4] mei: csc: add and enable polling option

[Alexander Usyskin]

Add polling option for CSC hardware with not working interrupt line.
Enable it by default as current hardware is broken.
Fix common mei-me polling code to avoid stalls and access to
powered down hardware.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
Alexander Usyskin (4):
      mei: make polling wait interruptible
      mei: csc: add option for polling and enable it
      mei: me: avoid hw access in polling when not active
      mei: me: reduce active polling timeout to 20 ms

 drivers/misc/mei/gsc-me.c  |  2 +-
 drivers/misc/mei/hw-me.c   | 99 +++++++++++++++++++++++++++-------------------
 drivers/misc/mei/pci-csc.c | 71 +++++++++++++++++++++++++--------
 3 files changed, 115 insertions(+), 57 deletions(-)
---
base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
change-id: 20260831-cri_polling-92ce7f9061f2

Best regards,
-- 
Alexander Usyskin <alexander.usyskin@intel.com>

[PATCH char-misc-next 1/4] mei: make polling wait interruptible

[Alexander Usyskin]

Polling thread wait should be interruptible to avoid stuck task.

Reviewed-by: Menachem Adin <menachem.adin@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/gsc-me.c | 2 +-
 drivers/misc/mei/hw-me.c  | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/mei/gsc-me.c b/drivers/misc/mei/gsc-me.c
index 376840b2c354..b38ddb04f2e8 100644
--- a/drivers/misc/mei/gsc-me.c
+++ b/drivers/misc/mei/gsc-me.c
@@ -242,7 +242,7 @@ static int __maybe_unused mei_gsc_pm_runtime_resume(struct device *device)
 
 	if (mei_me_hw_use_polling(hw)) {
 		hw->is_active = true;
-		wake_up(&hw->wait_active);
+		wake_up_interruptible(&hw->wait_active);
 	}
 
 	mutex_unlock(&dev->device_lock);
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index e7fbc02fb70f..ef7e1289ad7e 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -1441,9 +1441,9 @@ int mei_me_polling_thread(void *_dev)
 		struct mei_me_hw *hw = to_me_hw(dev);
 		u32 hcsr;
 
-		wait_event_timeout(hw->wait_active,
-				   hw->is_active || kthread_should_stop(),
-				   msecs_to_jiffies(MEI_POLLING_TIMEOUT_IDLE));
+		wait_event_interruptible_timeout(hw->wait_active,
+						 hw->is_active || kthread_should_stop(),
+						 msecs_to_jiffies(MEI_POLLING_TIMEOUT_IDLE));
 
 		if (kthread_should_stop())
 			break;

-- 
2.53.0

[PATCH char-misc-next 2/4] mei: csc: add option for polling and enable it

[Alexander Usyskin]

Add option to poll instead of waiting for the interrupt
to wallpaper over hardware not releasing the interrupt line.

Enable this workaround and leave option to use interrupts when
hardware is fixed.

Reviewed-by: Menachem Adin <menachem.adin@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/pci-csc.c | 71 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 55 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/mei/pci-csc.c b/drivers/misc/mei/pci-csc.c
index 6e1e8008f267..f50ef93e27dc 100644
--- a/drivers/misc/mei/pci-csc.c
+++ b/drivers/misc/mei/pci-csc.c
@@ -11,6 +11,7 @@
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
+#include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/pci.h>
@@ -26,6 +27,10 @@
 
 #define MEI_CSC_HECI2_OFFSET 0x1000
 
+static bool use_polling = true;
+module_param(use_polling, bool, 0600);
+MODULE_PARM_DESC(use_polling, "Use polling instead of interrupts");
+
 static int mei_csc_read_fws(const struct mei_device *mdev, int where, const char *name, u32 *val)
 {
 	struct mei_me_hw *hw = to_me_hw(mdev);
@@ -87,20 +92,40 @@ static int mei_csc_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, mdev);
 
-	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_INTX | PCI_IRQ_MSI);
-	if (err < 0) {
-		dev_err_probe(dev, err, "Failed to allocate IRQ.\n");
-		goto err_mei_unreg;
+	if (use_polling) {
+		dev_dbg(dev, "Using polling thread\n");
+		hw->irq = -1;
 	}
 
-	hw->irq = pci_irq_vector(pdev, 0);
-
-	/* request and enable interrupt */
-	err = request_threaded_irq(hw->irq,
-				   mei_me_irq_quick_handler, mei_me_irq_thread_handler,
-				   IRQF_SHARED | IRQF_ONESHOT, KBUILD_MODNAME, mdev);
-	if (err)
-		goto err_free_irq_vectors;
+	/* use polling */
+	if (mei_me_hw_use_polling(hw)) {
+		mei_disable_interrupts(mdev);
+		mei_clear_interrupts(mdev);
+		init_waitqueue_head(&hw->wait_active);
+		hw->is_active = true; /* start in active mode for initialization */
+		hw->polling_thread = kthread_run(mei_me_polling_thread, mdev,
+						 "kmecscirqd/%s", dev_name(dev));
+		if (IS_ERR(hw->polling_thread)) {
+			err = PTR_ERR(hw->polling_thread);
+			dev_err_probe(dev, err, "unable to create kernel thread.\n");
+			goto err_mei_unreg;
+		}
+	} else {
+		err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_INTX | PCI_IRQ_MSI);
+		if (err < 0) {
+			dev_err_probe(dev, err, "Failed to allocate IRQ.\n");
+			goto err_mei_unreg;
+		}
+
+		hw->irq = pci_irq_vector(pdev, 0);
+
+		/* request and enable interrupt */
+		err = request_threaded_irq(hw->irq,
+					   mei_me_irq_quick_handler, mei_me_irq_thread_handler,
+					   IRQF_SHARED | IRQF_ONESHOT, KBUILD_MODNAME, mdev);
+		if (err)
+			goto err_free_irq_vectors;
+	}
 
 	/*
 	 * Continue to char device setup in spite of firmware handshake failure.
@@ -126,7 +151,8 @@ static int mei_csc_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 
 err_free_irq_vectors:
-	pci_free_irq_vectors(pdev);
+	if (!mei_me_hw_use_polling(hw))
+		pci_free_irq_vectors(pdev);
 err_mei_unreg:
 	mei_deregister(mdev);
 	return err;
@@ -141,9 +167,14 @@ static void mei_csc_shutdown(struct pci_dev *pdev)
 
 	mei_stop(mdev);
 
+	if (mei_me_hw_use_polling(hw))
+		kthread_stop(hw->polling_thread);
+
 	mei_disable_interrupts(mdev);
-	free_irq(hw->irq, mdev);
-	pci_free_irq_vectors(pdev);
+	if (!mei_me_hw_use_polling(hw)) {
+		free_irq(hw->irq, mdev);
+		pci_free_irq_vectors(pdev);
+	}
 }
 
 static void mei_csc_remove(struct pci_dev *pdev)
@@ -210,6 +241,9 @@ static int mei_csc_pm_runtime_suspend(struct device *dev)
 		return -EAGAIN;
 
 	hw->pg_state = MEI_PG_ON;
+	if (mei_me_hw_use_polling(hw))
+		hw->is_active = false;
+
 	return 0;
 }
 
@@ -219,8 +253,13 @@ static int mei_csc_pm_runtime_resume(struct device *dev)
 	struct mei_me_hw *hw = to_me_hw(mdev);
 	irqreturn_t irq_ret;
 
-	scoped_guard(mutex, &mdev->device_lock)
+	scoped_guard(mutex, &mdev->device_lock) {
 		hw->pg_state = MEI_PG_OFF;
+		if (mei_me_hw_use_polling(hw)) {
+			hw->is_active = true;
+			wake_up_interruptible(&hw->wait_active);
+		}
+	}
 
 	/* Process all queues that wait for resume */
 	irq_ret = mei_me_irq_thread_handler(1, mdev);

-- 
2.53.0

[PATCH char-misc-next 3/4] mei: me: avoid hw access in polling when not active

[Alexander Usyskin]

Polling thread should not access hardware when not active.
Check for active state under device lock to avoid such
access when hardware is powered down.

Reviewed-by: Menachem Adin <menachem.adin@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/hw-me.c | 99 +++++++++++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 40 deletions(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index ef7e1289ad7e..f3d833be0e1a 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/kthread.h>
@@ -1290,28 +1291,15 @@ irqreturn_t mei_me_irq_quick_handler(int irq, void *dev_id)
 }
 EXPORT_SYMBOL_GPL(mei_me_irq_quick_handler);
 
-/**
- * mei_me_irq_thread_handler - function called after ISR to handle the interrupt
- * processing.
- *
- * @irq: The irq number
- * @dev_id: pointer to the device structure
- *
- * Return: irqreturn_t
- *
- */
-irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
+static irqreturn_t __mei_me_irq_thread_handler(struct mei_device *dev)
 {
-	struct mei_device *dev = (struct mei_device *) dev_id;
 	struct list_head cmpl_list;
 	bool pg_blocked;
 	s32 slots;
 	u32 hcsr;
 	int rets = 0;
 
-	dev_dbg(&dev->dev, "function called after ISR to handle the interrupt processing.\n");
 	/* initialize our complete list */
-	mutex_lock(&dev->device_lock);
 
 	hcsr = mei_hcsr_read(dev);
 	me_intr_clear(dev, hcsr);
@@ -1406,13 +1394,33 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 end:
 	dev_dbg(&dev->dev, "interrupt thread end ret = %d\n", rets);
 	mei_me_intr_enable(dev);
-	mutex_unlock(&dev->device_lock);
 	return IRQ_HANDLED;
 }
+
+/**
+ * mei_me_irq_thread_handler - function called after ISR to handle the interrupt
+ * processing.
+ *
+ * @irq: The irq number
+ * @dev_id: pointer to the device structure
+ *
+ * Return: irqreturn_t
+ *
+ */
+irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
+{
+	struct mei_device *dev = (struct mei_device *) dev_id;
+
+	dev_dbg(&dev->dev, "function called after ISR to handle the interrupt processing.\n");
+
+	guard(mutex)(&dev->device_lock);
+
+	return __mei_me_irq_thread_handler(dev);
+}
 EXPORT_SYMBOL_GPL(mei_me_irq_thread_handler);
 
-#define MEI_POLLING_TIMEOUT_ACTIVE 100
-#define MEI_POLLING_TIMEOUT_IDLE   500
+#define MEI_POLL_ACTIVE_MS 100
+#define MEI_POLL_IDLE_MS   500
 
 /**
  * mei_me_polling_thread - interrupt register polling thread
@@ -1420,48 +1428,59 @@ EXPORT_SYMBOL_GPL(mei_me_irq_thread_handler);
  * @_dev: mei device
  *
  * The thread monitors the interrupt source register and calls
- * mei_me_irq_thread_handler() to handle the firmware
+ * __mei_me_irq_thread_handler() to handle the firmware
  * input.
  *
- * The function polls in MEI_POLLING_TIMEOUT_ACTIVE timeout
+ * The function polls in MEI_POLL_ACTIVE_MS timeout
  * in case there was an event, in idle case the polling
- * time increases yet again by MEI_POLLING_TIMEOUT_ACTIVE
- * up to MEI_POLLING_TIMEOUT_IDLE.
+ * time increases yet again by MEI_POLL_ACTIVE_MS
+ * up to MEI_POLL_IDLE_MS.
  *
  * Return: always 0
  */
 int mei_me_polling_thread(void *_dev)
 {
 	struct mei_device *dev = _dev;
-	irqreturn_t irq_ret;
-	long polling_timeout = MEI_POLLING_TIMEOUT_ACTIVE;
+	long polling_timeout = MEI_POLL_ACTIVE_MS;
+	bool have_interrupt;
 
 	dev_dbg(&dev->dev, "kernel thread is running\n");
 	while (!kthread_should_stop()) {
 		struct mei_me_hw *hw = to_me_hw(dev);
 		u32 hcsr;
 
-		wait_event_interruptible_timeout(hw->wait_active,
-						 hw->is_active || kthread_should_stop(),
-						 msecs_to_jiffies(MEI_POLLING_TIMEOUT_IDLE));
+		if (!hw->is_active)
+			wait_event_interruptible(hw->wait_active,
+						 hw->is_active || kthread_should_stop());
+		else
+			wait_event_interruptible_timeout(hw->wait_active,
+							 hw->is_active || kthread_should_stop(),
+							 msecs_to_jiffies(MEI_POLL_IDLE_MS));
 
 		if (kthread_should_stop())
 			break;
 
-		hcsr = mei_hcsr_read(dev);
-		if (me_intr_src(hcsr)) {
-			polling_timeout = MEI_POLLING_TIMEOUT_ACTIVE;
-			irq_ret = mei_me_irq_thread_handler(1, dev);
-			if (irq_ret != IRQ_HANDLED)
-				dev_err(&dev->dev, "irq_ret %d\n", irq_ret);
-		} else {
-			/*
-			 * Increase timeout by MEI_POLLING_TIMEOUT_ACTIVE
-			 * up to MEI_POLLING_TIMEOUT_IDLE
-			 */
-			polling_timeout = clamp_val(polling_timeout + MEI_POLLING_TIMEOUT_ACTIVE,
-						    MEI_POLLING_TIMEOUT_ACTIVE,
-						    MEI_POLLING_TIMEOUT_IDLE);
+		scoped_guard(mutex, &dev->device_lock) {
+			if (!hw->is_active) {
+				have_interrupt = false;
+			} else {
+				hcsr = mei_hcsr_read(dev);
+				have_interrupt = !!me_intr_src(hcsr);
+			}
+
+			if (have_interrupt) {
+				polling_timeout = MEI_POLL_ACTIVE_MS;
+				dev_dbg(&dev->dev, "polling interrupt processing\n");
+				__mei_me_irq_thread_handler(dev);
+			} else {
+				/*
+				 * Increase timeout by MEI_POLL_ACTIVE_MS
+				 * up to MEI_POLL_IDLE_MS
+				 */
+				polling_timeout = clamp_val(polling_timeout + MEI_POLL_ACTIVE_MS,
+							    MEI_POLL_ACTIVE_MS,
+							    MEI_POLL_IDLE_MS);
+			}
 		}
 
 		schedule_timeout_interruptible(msecs_to_jiffies(polling_timeout));

-- 
2.53.0

[PATCH char-misc-next 4/4] mei: me: reduce active polling timeout to 20 ms

[Alexander Usyskin]

Improve performance of polling by reducing active timeout.
20 ms active timeout helps increase channel throughtput
and significately reduces data transfer times.

Our tests have prooved that timeout decrease does no ill effects
for core CPU utilization.

Reviewed-by: Menachem Adin <menachem.adin@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/hw-me.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index f3d833be0e1a..1c3ab9eb8b1b 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -1419,7 +1419,7 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 }
 EXPORT_SYMBOL_GPL(mei_me_irq_thread_handler);
 
-#define MEI_POLL_ACTIVE_MS 100
+#define MEI_POLL_ACTIVE_MS 20
 #define MEI_POLL_IDLE_MS   500
 
 /**

-- 
2.53.0