linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] mei: migrate to pci_alloc_irq_vectors and smaller improvements
@ 2018-08-03 21:37 Heiner Kallweit
  2018-08-03 21:39 ` [PATCH 1/3] mei: Don't free irq when suspending Heiner Kallweit
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Heiner Kallweit @ 2018-08-03 21:37 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Kernel Mailing List

This series migrates the driver to the pci_alloc_irq_vectors API and
includes also smaller improvements.

MEI_ME was tested on a Zotac CI321. MEI_TXE is compile-tested only.

Heiner Kallweit (3):
  mei: Don't free irq when suspending
  mei: Migrate to pci_alloc_irq_vectors API
  mei: Improve usage of mei_start

 drivers/misc/mei/pci-me.c  | 51 ++++++++----------------------
 drivers/misc/mei/pci-txe.c | 63 ++++++++++----------------------------
 2 files changed, 30 insertions(+), 84 deletions(-)

-- 
2.18.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] mei: Don't free irq when suspending
  2018-08-03 21:37 [PATCH 0/3] mei: migrate to pci_alloc_irq_vectors and smaller improvements Heiner Kallweit
@ 2018-08-03 21:39 ` Heiner Kallweit
  2018-08-03 21:40 ` [PATCH 2/3] mei: Migrate to pci_alloc_irq_vectors API Heiner Kallweit
  2018-08-03 21:42 ` [PATCH 3/3] mei: Improve usage of mei_start Heiner Kallweit
  2 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2018-08-03 21:39 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Kernel Mailing List

Usually it's not needed to free and re-request the interrupt in a
suspend / resume cycle, so remove this.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/mei/pci-me.c  | 20 --------------------
 drivers/misc/mei/pci-txe.c | 27 +--------------------------
 2 files changed, 1 insertion(+), 46 deletions(-)

diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index ea4e1522..55ba1c41 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -336,9 +336,6 @@ static int mei_me_pci_suspend(struct device *device)
 
 	mei_disable_interrupts(dev);
 
-	free_irq(pdev->irq, dev);
-	pci_disable_msi(pdev);
-
 	return 0;
 }
 
@@ -346,29 +343,12 @@ static int mei_me_pci_resume(struct device *device)
 {
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct mei_device *dev;
-	unsigned int irqflags;
 	int err;
 
 	dev = pci_get_drvdata(pdev);
 	if (!dev)
 		return -ENODEV;
 
-	pci_enable_msi(pdev);
-
-	irqflags = pci_dev_msi_enabled(pdev) ? IRQF_ONESHOT : IRQF_SHARED;
-
-	/* request and enable interrupt */
-	err = request_threaded_irq(pdev->irq,
-			mei_me_irq_quick_handler,
-			mei_me_irq_thread_handler,
-			irqflags, KBUILD_MODNAME, dev);
-
-	if (err) {
-		dev_err(&pdev->dev, "request_threaded_irq failed: irq = %d.\n",
-				pdev->irq);
-		return err;
-	}
-
 	err = mei_restart(dev);
 	if (err)
 		return err;
diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c
index e1b90912..b8a49581 100644
--- a/drivers/misc/mei/pci-txe.c
+++ b/drivers/misc/mei/pci-txe.c
@@ -240,9 +240,6 @@ static int mei_txe_pci_suspend(struct device *device)
 
 	mei_disable_interrupts(dev);
 
-	free_irq(pdev->irq, dev);
-	pci_disable_msi(pdev);
-
 	return 0;
 }
 
@@ -250,36 +247,14 @@ static int mei_txe_pci_resume(struct device *device)
 {
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct mei_device *dev;
-	int err;
 
 	dev = pci_get_drvdata(pdev);
 	if (!dev)
 		return -ENODEV;
 
-	pci_enable_msi(pdev);
-
 	mei_clear_interrupts(dev);
 
-	/* request and enable interrupt */
-	if (pci_dev_msi_enabled(pdev))
-		err = request_threaded_irq(pdev->irq,
-			NULL,
-			mei_txe_irq_thread_handler,
-			IRQF_ONESHOT, KBUILD_MODNAME, dev);
-	else
-		err = request_threaded_irq(pdev->irq,
-			mei_txe_irq_quick_handler,
-			mei_txe_irq_thread_handler,
-			IRQF_SHARED, KBUILD_MODNAME, dev);
-	if (err) {
-		dev_err(&pdev->dev, "request_threaded_irq failed: irq = %d.\n",
-				pdev->irq);
-		return err;
-	}
-
-	err = mei_restart(dev);
-
-	return err;
+	return mei_restart(dev);
 }
 #endif /* CONFIG_PM_SLEEP */
 
-- 
2.18.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] mei: Migrate to pci_alloc_irq_vectors API
  2018-08-03 21:37 [PATCH 0/3] mei: migrate to pci_alloc_irq_vectors and smaller improvements Heiner Kallweit
  2018-08-03 21:39 ` [PATCH 1/3] mei: Don't free irq when suspending Heiner Kallweit
@ 2018-08-03 21:40 ` Heiner Kallweit
  2018-08-05 11:02   ` Winkler, Tomas
  2018-08-03 21:42 ` [PATCH 3/3] mei: Improve usage of mei_start Heiner Kallweit
  2 siblings, 1 reply; 6+ messages in thread
From: Heiner Kallweit @ 2018-08-03 21:40 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Kernel Mailing List

Update the driver to the pci_alloc_irq_vectors API, this allows to get
rid of legacy calls like pci_enable_msi(). Another benefit is that
no driver change would be needed in case the hardware starts
supporting MSI-X.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/mei/pci-me.c  | 25 +++++++++++--------------
 drivers/misc/mei/pci-txe.c | 30 ++++++++++++++----------------
 2 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 55ba1c41..a60376f0 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -149,7 +149,6 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	const struct mei_cfg *cfg;
 	struct mei_device *dev;
 	struct mei_me_hw *hw;
-	unsigned int irqflags;
 	int err;
 
 	cfg = mei_me_get_cfg(ent->driver_data);
@@ -196,18 +195,16 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	hw = to_me_hw(dev);
 	hw->mem_addr = pcim_iomap_table(pdev)[0];
 
-	pci_enable_msi(pdev);
-
-	 /* request and enable interrupt */
-	irqflags = pci_dev_msi_enabled(pdev) ? IRQF_ONESHOT : IRQF_SHARED;
+	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (err < 0)
+		goto end;
 
-	err = request_threaded_irq(pdev->irq,
-			mei_me_irq_quick_handler,
-			mei_me_irq_thread_handler,
-			irqflags, KBUILD_MODNAME, dev);
+	/* request and enable interrupt */
+	err = pci_request_irq(pdev, 0, mei_me_irq_quick_handler,
+			      mei_me_irq_thread_handler, dev, KBUILD_MODNAME);
 	if (err) {
-		dev_err(&pdev->dev, "request_threaded_irq failure. irq = %d\n",
-		       pdev->irq);
+		pci_err(pdev, "pci_request_irq failure. irq = %d\n",
+			pci_irq_vector(pdev, 0));
 		goto end;
 	}
 
@@ -258,7 +255,7 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 release_irq:
 	mei_cancel_work(dev);
 	mei_disable_interrupts(dev);
-	free_irq(pdev->irq, dev);
+	pci_free_irq(pdev, 0, dev);
 end:
 	dev_err(&pdev->dev, "initialization failed.\n");
 	return err;
@@ -287,7 +284,7 @@ static void mei_me_shutdown(struct pci_dev *pdev)
 	mei_me_unset_pm_domain(dev);
 
 	mei_disable_interrupts(dev);
-	free_irq(pdev->irq, dev);
+	pci_free_irq(pdev, 0, dev);
 }
 
 /**
@@ -316,7 +313,7 @@ static void mei_me_remove(struct pci_dev *pdev)
 
 	mei_disable_interrupts(dev);
 
-	free_irq(pdev->irq, dev);
+	pci_free_irq(pdev, 0, dev);
 
 	mei_deregister(dev);
 }
diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c
index b8a49581..3ea2b8f6 100644
--- a/drivers/misc/mei/pci-txe.c
+++ b/drivers/misc/mei/pci-txe.c
@@ -64,6 +64,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct mei_device *dev;
 	struct mei_txe_hw *hw;
+	irq_handler_t qh = NULL;
 	const int mask = BIT(SEC_BAR) | BIT(BRIDGE_BAR);
 	int err;
 
@@ -100,25 +101,22 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	hw = to_txe_hw(dev);
 	hw->mem_addr = pcim_iomap_table(pdev);
 
-	pci_enable_msi(pdev);
+	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (err < 0)
+		goto end;
 
 	/* clear spurious interrupts */
 	mei_clear_interrupts(dev);
 
+	if (!pci_dev_msi_enabled(pdev))
+		qh = mei_txe_irq_quick_handler;
+
 	/* request and enable interrupt  */
-	if (pci_dev_msi_enabled(pdev))
-		err = request_threaded_irq(pdev->irq,
-			NULL,
-			mei_txe_irq_thread_handler,
-			IRQF_ONESHOT, KBUILD_MODNAME, dev);
-	else
-		err = request_threaded_irq(pdev->irq,
-			mei_txe_irq_quick_handler,
-			mei_txe_irq_thread_handler,
-			IRQF_SHARED, KBUILD_MODNAME, dev);
+	err = pci_request_irq(pdev, 0, qh, mei_txe_irq_thread_handler,
+			      dev, KBUILD_MODNAME);
 	if (err) {
-		dev_err(&pdev->dev, "mei: request_threaded_irq failure. irq = %d\n",
-			pdev->irq);
+		pci_err(pdev, "mei: pci_request_irq failure. irq = %d\n",
+			pci_irq_vector(pdev, 0));
 		goto end;
 	}
 
@@ -162,7 +160,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 release_irq:
 	mei_cancel_work(dev);
 	mei_disable_interrupts(dev);
-	free_irq(pdev->irq, dev);
+	pci_free_irq(pdev, 0, dev);
 end:
 	dev_err(&pdev->dev, "initialization failed.\n");
 	return err;
@@ -191,7 +189,7 @@ static void mei_txe_shutdown(struct pci_dev *pdev)
 	mei_txe_unset_pm_domain(dev);
 
 	mei_disable_interrupts(dev);
-	free_irq(pdev->irq, dev);
+	pci_free_irq(pdev, 0, dev);
 }
 
 /**
@@ -219,7 +217,7 @@ static void mei_txe_remove(struct pci_dev *pdev)
 	mei_txe_unset_pm_domain(dev);
 
 	mei_disable_interrupts(dev);
-	free_irq(pdev->irq, dev);
+	pci_free_irq(pdev, 0, dev);
 
 	mei_deregister(dev);
 }
-- 
2.18.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] mei: Improve usage of mei_start
  2018-08-03 21:37 [PATCH 0/3] mei: migrate to pci_alloc_irq_vectors and smaller improvements Heiner Kallweit
  2018-08-03 21:39 ` [PATCH 1/3] mei: Don't free irq when suspending Heiner Kallweit
  2018-08-03 21:40 ` [PATCH 2/3] mei: Migrate to pci_alloc_irq_vectors API Heiner Kallweit
@ 2018-08-03 21:42 ` Heiner Kallweit
  2 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2018-08-03 21:42 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Kernel Mailing List

mei_start() prints an error message and returns -ENODEV on failure,
so we don't have to duplicate this in the caller.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/mei/pci-me.c  | 6 ++----
 drivers/misc/mei/pci-txe.c | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index a60376f0..02ba830a 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -208,11 +208,9 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto end;
 	}
 
-	if (mei_start(dev)) {
-		dev_err(&pdev->dev, "init hw failure.\n");
-		err = -ENODEV;
+	err = mei_start(dev);
+	if (err)
 		goto release_irq;
-	}
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, MEI_ME_RPM_TIMEOUT);
 	pm_runtime_use_autosuspend(&pdev->dev);
diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c
index 3ea2b8f6..901574be 100644
--- a/drivers/misc/mei/pci-txe.c
+++ b/drivers/misc/mei/pci-txe.c
@@ -120,11 +120,9 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto end;
 	}
 
-	if (mei_start(dev)) {
-		dev_err(&pdev->dev, "init hw failure.\n");
-		err = -ENODEV;
+	err = mei_start(dev);
+	if (err)
 		goto release_irq;
-	}
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, MEI_TXI_RPM_TIMEOUT);
 	pm_runtime_use_autosuspend(&pdev->dev);
-- 
2.18.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH 2/3] mei: Migrate to pci_alloc_irq_vectors API
  2018-08-03 21:40 ` [PATCH 2/3] mei: Migrate to pci_alloc_irq_vectors API Heiner Kallweit
@ 2018-08-05 11:02   ` Winkler, Tomas
  2018-08-05 11:18     ` Heiner Kallweit
  0 siblings, 1 reply; 6+ messages in thread
From: Winkler, Tomas @ 2018-08-05 11:02 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Kernel Mailing List

> 
> Update the driver to the pci_alloc_irq_vectors API, this allows to get rid of
> legacy calls like pci_enable_msi(). Another benefit is that no driver change
> would be needed in case the hardware starts supporting MSI-X.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Sorry, we are losing the ONE_SHOT flag here,  In spite that pci_enable_msi is marked as deprecated,  
I don't see overall action on LKML to actually  deprecate it.
For this change I would need to extensice testing on future and legacy devices, frankly  I prefer not to dig into it right now.
Thanks
Tomas

> ---
>  drivers/misc/mei/pci-me.c  | 25 +++++++++++--------------
> drivers/misc/mei/pci-txe.c | 30 ++++++++++++++----------------
>  2 files changed, 25 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c index
> 55ba1c41..a60376f0 100644
> --- a/drivers/misc/mei/pci-me.c
> +++ b/drivers/misc/mei/pci-me.c
> @@ -149,7 +149,6 @@ static int mei_me_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>  	const struct mei_cfg *cfg;
>  	struct mei_device *dev;
>  	struct mei_me_hw *hw;
> -	unsigned int irqflags;
>  	int err;
> 
>  	cfg = mei_me_get_cfg(ent->driver_data); @@ -196,18 +195,16 @@
> static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id
> *ent)
>  	hw = to_me_hw(dev);
>  	hw->mem_addr = pcim_iomap_table(pdev)[0];
> 
> -	pci_enable_msi(pdev);
> -
> -	 /* request and enable interrupt */
> -	irqflags = pci_dev_msi_enabled(pdev) ? IRQF_ONESHOT :
> IRQF_SHARED;
> +	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> +	if (err < 0)
> +		goto end;
> 
> -	err = request_threaded_irq(pdev->irq,
> -			mei_me_irq_quick_handler,
> -			mei_me_irq_thread_handler,
> -			irqflags, KBUILD_MODNAME, dev);
> +	/* request and enable interrupt */
> +	err = pci_request_irq(pdev, 0, mei_me_irq_quick_handler,
> +			      mei_me_irq_thread_handler, dev,
> KBUILD_MODNAME);
>  	if (err) {
> -		dev_err(&pdev->dev, "request_threaded_irq failure. irq =
> %d\n",
> -		       pdev->irq);
> +		pci_err(pdev, "pci_request_irq failure. irq = %d\n",
> +			pci_irq_vector(pdev, 0));
>  		goto end;
>  	}
> 
> @@ -258,7 +255,7 @@ static int mei_me_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>  release_irq:
>  	mei_cancel_work(dev);
>  	mei_disable_interrupts(dev);
> -	free_irq(pdev->irq, dev);
> +	pci_free_irq(pdev, 0, dev);
>  end:
>  	dev_err(&pdev->dev, "initialization failed.\n");
>  	return err;
> @@ -287,7 +284,7 @@ static void mei_me_shutdown(struct pci_dev *pdev)
>  	mei_me_unset_pm_domain(dev);
> 
>  	mei_disable_interrupts(dev);
> -	free_irq(pdev->irq, dev);
> +	pci_free_irq(pdev, 0, dev);
>  }
> 
>  /**
> @@ -316,7 +313,7 @@ static void mei_me_remove(struct pci_dev *pdev)
> 
>  	mei_disable_interrupts(dev);
> 
> -	free_irq(pdev->irq, dev);
> +	pci_free_irq(pdev, 0, dev);
> 
>  	mei_deregister(dev);
>  }
> diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c index
> b8a49581..3ea2b8f6 100644
> --- a/drivers/misc/mei/pci-txe.c
> +++ b/drivers/misc/mei/pci-txe.c
> @@ -64,6 +64,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)  {
>  	struct mei_device *dev;
>  	struct mei_txe_hw *hw;
> +	irq_handler_t qh = NULL;
>  	const int mask = BIT(SEC_BAR) | BIT(BRIDGE_BAR);
>  	int err;
> 
> @@ -100,25 +101,22 @@ static int mei_txe_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
>  	hw = to_txe_hw(dev);
>  	hw->mem_addr = pcim_iomap_table(pdev);
> 
> -	pci_enable_msi(pdev);
> +	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> +	if (err < 0)
> +		goto end;
> 
>  	/* clear spurious interrupts */
>  	mei_clear_interrupts(dev);
> 
> +	if (!pci_dev_msi_enabled(pdev))
> +		qh = mei_txe_irq_quick_handler;
> +
>  	/* request and enable interrupt  */
> -	if (pci_dev_msi_enabled(pdev))
> -		err = request_threaded_irq(pdev->irq,
> -			NULL,
> -			mei_txe_irq_thread_handler,
> -			IRQF_ONESHOT, KBUILD_MODNAME, dev);
> -	else
> -		err = request_threaded_irq(pdev->irq,
> -			mei_txe_irq_quick_handler,
> -			mei_txe_irq_thread_handler,
> -			IRQF_SHARED, KBUILD_MODNAME, dev);
> +	err = pci_request_irq(pdev, 0, qh, mei_txe_irq_thread_handler,
> +			      dev, KBUILD_MODNAME);
>  	if (err) {
> -		dev_err(&pdev->dev, "mei: request_threaded_irq failure. irq
> = %d\n",
> -			pdev->irq);
> +		pci_err(pdev, "mei: pci_request_irq failure. irq = %d\n",
> +			pci_irq_vector(pdev, 0));
>  		goto end;
>  	}
> 
> @@ -162,7 +160,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>  release_irq:
>  	mei_cancel_work(dev);
>  	mei_disable_interrupts(dev);
> -	free_irq(pdev->irq, dev);
> +	pci_free_irq(pdev, 0, dev);
>  end:
>  	dev_err(&pdev->dev, "initialization failed.\n");
>  	return err;
> @@ -191,7 +189,7 @@ static void mei_txe_shutdown(struct pci_dev *pdev)
>  	mei_txe_unset_pm_domain(dev);
> 
>  	mei_disable_interrupts(dev);
> -	free_irq(pdev->irq, dev);
> +	pci_free_irq(pdev, 0, dev);
>  }
> 
>  /**
> @@ -219,7 +217,7 @@ static void mei_txe_remove(struct pci_dev *pdev)
>  	mei_txe_unset_pm_domain(dev);
> 
>  	mei_disable_interrupts(dev);
> -	free_irq(pdev->irq, dev);
> +	pci_free_irq(pdev, 0, dev);
> 
>  	mei_deregister(dev);
>  }
> --
> 2.18.0
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] mei: Migrate to pci_alloc_irq_vectors API
  2018-08-05 11:02   ` Winkler, Tomas
@ 2018-08-05 11:18     ` Heiner Kallweit
  0 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2018-08-05 11:18 UTC (permalink / raw)
  To: Winkler, Tomas
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Linux Kernel Mailing List

On 05.08.2018 13:02, Winkler, Tomas wrote:
>>
>> Update the driver to the pci_alloc_irq_vectors API, this allows to get rid of
>> legacy calls like pci_enable_msi(). Another benefit is that no driver change
>> would be needed in case the hardware starts supporting MSI-X.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Sorry, we are losing the ONE_SHOT flag here,  In spite that pci_enable_msi is marked as deprecated,  
ONE_SHOT flag isn't needed here. See commit f7368a550275 ("PCI: Use IRQF_ONESHOT if
pci_request_irq() called with no handler") and also following comment in __setup_irq():

* Drivers are often written to work w/o knowledge about the
* underlying irq chip implementation, so a request for a
* threaded irq without a primary hard irq context handler
* requires the ONESHOT flag to be set. Some irq chips like
* MSI based interrupts are per se one shot safe. Check the
* chip flags, so we can avoid the unmask dance at the end of
* the threaded handler for those.

> I don't see overall action on LKML to actually  deprecate it.
> For this change I would need to extensice testing on future and legacy devices, frankly  I prefer not to dig into it right now.

Sure, up to you.

> Thanks
> Tomas
> 
Rgds, Heiner

>> ---
>>  drivers/misc/mei/pci-me.c  | 25 +++++++++++--------------
>> drivers/misc/mei/pci-txe.c | 30 ++++++++++++++----------------
>>  2 files changed, 25 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c index
>> 55ba1c41..a60376f0 100644
>> --- a/drivers/misc/mei/pci-me.c
>> +++ b/drivers/misc/mei/pci-me.c
>> @@ -149,7 +149,6 @@ static int mei_me_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>>  	const struct mei_cfg *cfg;
>>  	struct mei_device *dev;
>>  	struct mei_me_hw *hw;
>> -	unsigned int irqflags;
>>  	int err;
>>
>>  	cfg = mei_me_get_cfg(ent->driver_data); @@ -196,18 +195,16 @@
>> static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id
>> *ent)
>>  	hw = to_me_hw(dev);
>>  	hw->mem_addr = pcim_iomap_table(pdev)[0];
>>
>> -	pci_enable_msi(pdev);
>> -
>> -	 /* request and enable interrupt */
>> -	irqflags = pci_dev_msi_enabled(pdev) ? IRQF_ONESHOT :
>> IRQF_SHARED;
>> +	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
>> +	if (err < 0)
>> +		goto end;
>>
>> -	err = request_threaded_irq(pdev->irq,
>> -			mei_me_irq_quick_handler,
>> -			mei_me_irq_thread_handler,
>> -			irqflags, KBUILD_MODNAME, dev);
>> +	/* request and enable interrupt */
>> +	err = pci_request_irq(pdev, 0, mei_me_irq_quick_handler,
>> +			      mei_me_irq_thread_handler, dev,
>> KBUILD_MODNAME);
>>  	if (err) {
>> -		dev_err(&pdev->dev, "request_threaded_irq failure. irq =
>> %d\n",
>> -		       pdev->irq);
>> +		pci_err(pdev, "pci_request_irq failure. irq = %d\n",
>> +			pci_irq_vector(pdev, 0));
>>  		goto end;
>>  	}
>>
>> @@ -258,7 +255,7 @@ static int mei_me_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>>  release_irq:
>>  	mei_cancel_work(dev);
>>  	mei_disable_interrupts(dev);
>> -	free_irq(pdev->irq, dev);
>> +	pci_free_irq(pdev, 0, dev);
>>  end:
>>  	dev_err(&pdev->dev, "initialization failed.\n");
>>  	return err;
>> @@ -287,7 +284,7 @@ static void mei_me_shutdown(struct pci_dev *pdev)
>>  	mei_me_unset_pm_domain(dev);
>>
>>  	mei_disable_interrupts(dev);
>> -	free_irq(pdev->irq, dev);
>> +	pci_free_irq(pdev, 0, dev);
>>  }
>>
>>  /**
>> @@ -316,7 +313,7 @@ static void mei_me_remove(struct pci_dev *pdev)
>>
>>  	mei_disable_interrupts(dev);
>>
>> -	free_irq(pdev->irq, dev);
>> +	pci_free_irq(pdev, 0, dev);
>>
>>  	mei_deregister(dev);
>>  }
>> diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c index
>> b8a49581..3ea2b8f6 100644
>> --- a/drivers/misc/mei/pci-txe.c
>> +++ b/drivers/misc/mei/pci-txe.c
>> @@ -64,6 +64,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)  {
>>  	struct mei_device *dev;
>>  	struct mei_txe_hw *hw;
>> +	irq_handler_t qh = NULL;
>>  	const int mask = BIT(SEC_BAR) | BIT(BRIDGE_BAR);
>>  	int err;
>>
>> @@ -100,25 +101,22 @@ static int mei_txe_probe(struct pci_dev *pdev,
>> const struct pci_device_id *ent)
>>  	hw = to_txe_hw(dev);
>>  	hw->mem_addr = pcim_iomap_table(pdev);
>>
>> -	pci_enable_msi(pdev);
>> +	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
>> +	if (err < 0)
>> +		goto end;
>>
>>  	/* clear spurious interrupts */
>>  	mei_clear_interrupts(dev);
>>
>> +	if (!pci_dev_msi_enabled(pdev))
>> +		qh = mei_txe_irq_quick_handler;
>> +
>>  	/* request and enable interrupt  */
>> -	if (pci_dev_msi_enabled(pdev))
>> -		err = request_threaded_irq(pdev->irq,
>> -			NULL,
>> -			mei_txe_irq_thread_handler,
>> -			IRQF_ONESHOT, KBUILD_MODNAME, dev);
>> -	else
>> -		err = request_threaded_irq(pdev->irq,
>> -			mei_txe_irq_quick_handler,
>> -			mei_txe_irq_thread_handler,
>> -			IRQF_SHARED, KBUILD_MODNAME, dev);
>> +	err = pci_request_irq(pdev, 0, qh, mei_txe_irq_thread_handler,
>> +			      dev, KBUILD_MODNAME);
>>  	if (err) {
>> -		dev_err(&pdev->dev, "mei: request_threaded_irq failure. irq
>> = %d\n",
>> -			pdev->irq);
>> +		pci_err(pdev, "mei: pci_request_irq failure. irq = %d\n",
>> +			pci_irq_vector(pdev, 0));
>>  		goto end;
>>  	}
>>
>> @@ -162,7 +160,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>>  release_irq:
>>  	mei_cancel_work(dev);
>>  	mei_disable_interrupts(dev);
>> -	free_irq(pdev->irq, dev);
>> +	pci_free_irq(pdev, 0, dev);
>>  end:
>>  	dev_err(&pdev->dev, "initialization failed.\n");
>>  	return err;
>> @@ -191,7 +189,7 @@ static void mei_txe_shutdown(struct pci_dev *pdev)
>>  	mei_txe_unset_pm_domain(dev);
>>
>>  	mei_disable_interrupts(dev);
>> -	free_irq(pdev->irq, dev);
>> +	pci_free_irq(pdev, 0, dev);
>>  }
>>
>>  /**
>> @@ -219,7 +217,7 @@ static void mei_txe_remove(struct pci_dev *pdev)
>>  	mei_txe_unset_pm_domain(dev);
>>
>>  	mei_disable_interrupts(dev);
>> -	free_irq(pdev->irq, dev);
>> +	pci_free_irq(pdev, 0, dev);
>>
>>  	mei_deregister(dev);
>>  }
>> --
>> 2.18.0
>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-08-05 11:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-03 21:37 [PATCH 0/3] mei: migrate to pci_alloc_irq_vectors and smaller improvements Heiner Kallweit
2018-08-03 21:39 ` [PATCH 1/3] mei: Don't free irq when suspending Heiner Kallweit
2018-08-03 21:40 ` [PATCH 2/3] mei: Migrate to pci_alloc_irq_vectors API Heiner Kallweit
2018-08-05 11:02   ` Winkler, Tomas
2018-08-05 11:18     ` Heiner Kallweit
2018-08-03 21:42 ` [PATCH 3/3] mei: Improve usage of mei_start Heiner Kallweit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).