[PATCH 0/4] firewire: ohci: obsolete usage of legacy API for IRQ request and MSI

[PATCH 0/4] firewire: ohci: obsolete usage of legacy API for IRQ request and MSI

[Takashi Sakamoto]

Hi,

The pair of pci_enable_msi() and pci_disable_msi() is deprecated
nowadays, while it is still used in 1394 OHCI driver in FireWire
subsystem. This series of changes is to use modern API sets for the
driver.


Takashi Sakamoto (4):
  Revert "firewire: ohci: use devres for requested IRQ"
  firewire: ohci: replace request_irq() with request_threaded_irq()
  firewire: ohci: obsolete usage of deprecated API for MSI
  firewire: ohci: use pci_irq_vector() to retrieve allocated interrupt
    line

 drivers/firewire/ohci.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

-- 
2.43.0

[PATCH 1/4] Revert "firewire: ohci: use devres for requested IRQ"

[Takashi Sakamoto]

This reverts commit 5a95f1ded28691e69f7d6718c5dcbc149613d431.

As long as allocating any device interrupt vector for MSI, it is
inconvenient to utilize managed device resources for IRQ requesting.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/ohci.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 7bc71f4be64a..f18019c5d220 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -3750,16 +3750,17 @@ static int pci_probe(struct pci_dev *dev,
 
 	if (!(ohci->quirks & QUIRK_NO_MSI))
 		pci_enable_msi(dev);
-	err = devm_request_irq(&dev->dev, dev->irq, irq_handler,
-			       pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, ohci_driver_name, ohci);
-	if (err < 0) {
+	if (request_irq(dev->irq, irq_handler,
+			pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED,
+			ohci_driver_name, ohci)) {
 		ohci_err(ohci, "failed to allocate interrupt %d\n", dev->irq);
+		err = -EIO;
 		goto fail_msi;
 	}
 
 	err = fw_card_add(&ohci->card, max_receive, link_speed, guid);
 	if (err)
-		goto fail_msi;
+		goto fail_irq;
 
 	version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff;
 	ohci_notice(ohci,
@@ -3772,8 +3773,9 @@ static int pci_probe(struct pci_dev *dev,
 
 	return 0;
 
+ fail_irq:
+	free_irq(dev->irq, ohci);
  fail_msi:
-	devm_free_irq(&dev->dev, dev->irq, ohci);
 	pci_disable_msi(dev);
 
 	return err;
@@ -3801,7 +3803,7 @@ static void pci_remove(struct pci_dev *dev)
 
 	software_reset(ohci);
 
-	devm_free_irq(&dev->dev, dev->irq, ohci);
+	free_irq(dev->irq, ohci);
 	pci_disable_msi(dev);
 
 	dev_notice(&dev->dev, "removing fw-ohci device\n");
-- 
2.43.0

[PATCH 2/4] firewire: ohci: replace request_irq() with request_threaded_irq()

[Takashi Sakamoto]

Nowadays request_irq() is a wrapper of request_threaded_irq(). The IRQ
handler of 1394 ohci driver has never been optimized yet, while it is
a good preparation for the future work to replace the latter.

This commit replaces the former.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/ohci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index f18019c5d220..ec02708c5f05 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -3750,11 +3750,11 @@ static int pci_probe(struct pci_dev *dev,
 
 	if (!(ohci->quirks & QUIRK_NO_MSI))
 		pci_enable_msi(dev);
-	if (request_irq(dev->irq, irq_handler,
-			pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED,
-			ohci_driver_name, ohci)) {
+	err = request_threaded_irq(dev->irq, irq_handler, NULL,
+				   pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, ohci_driver_name,
+				   ohci);
+	if (err < 0) {
 		ohci_err(ohci, "failed to allocate interrupt %d\n", dev->irq);
-		err = -EIO;
 		goto fail_msi;
 	}
 
-- 
2.43.0

[PATCH 3/4] firewire: ohci: obsolete usage of deprecated API for MSI

[Takashi Sakamoto]

The usage of the pair of pci_enable_msi() and pci_disable_msi() is
deprecated.

This commit uses the preferred pair of API for the purpose. The call of
pci_alloc_irq_vectors() can have a subeffect to change the return value
of pci_dev_msi_enabled().

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/ohci.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index ec02708c5f05..c14e6efaba36 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -3623,7 +3623,7 @@ static int pci_probe(struct pci_dev *dev,
 	struct fw_ohci *ohci;
 	u32 bus_options, max_receive, link_speed, version;
 	u64 guid;
-	int i, err;
+	int i, flags, err;
 	size_t size;
 
 	if (dev->vendor == PCI_VENDOR_ID_PINNACLE_SYSTEMS) {
@@ -3748,8 +3748,13 @@ static int pci_probe(struct pci_dev *dev,
 	guid = ((u64) reg_read(ohci, OHCI1394_GUIDHi) << 32) |
 		reg_read(ohci, OHCI1394_GUIDLo);
 
+	flags = PCI_IRQ_INTX;
 	if (!(ohci->quirks & QUIRK_NO_MSI))
-		pci_enable_msi(dev);
+		flags |= PCI_IRQ_MSI;
+	err = pci_alloc_irq_vectors(dev, 1, 1, flags);
+	if (err < 0)
+		return err;
+
 	err = request_threaded_irq(dev->irq, irq_handler, NULL,
 				   pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, ohci_driver_name,
 				   ohci);
@@ -3776,7 +3781,7 @@ static int pci_probe(struct pci_dev *dev,
  fail_irq:
 	free_irq(dev->irq, ohci);
  fail_msi:
-	pci_disable_msi(dev);
+	pci_free_irq_vectors(dev);
 
 	return err;
 }
@@ -3804,7 +3809,7 @@ static void pci_remove(struct pci_dev *dev)
 	software_reset(ohci);
 
 	free_irq(dev->irq, ohci);
-	pci_disable_msi(dev);
+	pci_free_irq_vectors(dev);
 
 	dev_notice(&dev->dev, "removing fw-ohci device\n");
 }
-- 
2.43.0

[PATCH 4/4] firewire: ohci: use pci_irq_vector() to retrieve allocated interrupt line

[Takashi Sakamoto]

The pci_irq_vector() is available to retrieve the allocated interrupt line
instead of the direct access to the member of device structure.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/ohci.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index c14e6efaba36..f3b610f86aac 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -3623,7 +3623,7 @@ static int pci_probe(struct pci_dev *dev,
 	struct fw_ohci *ohci;
 	u32 bus_options, max_receive, link_speed, version;
 	u64 guid;
-	int i, flags, err;
+	int i, flags, irq, err;
 	size_t size;
 
 	if (dev->vendor == PCI_VENDOR_ID_PINNACLE_SYSTEMS) {
@@ -3754,12 +3754,17 @@ static int pci_probe(struct pci_dev *dev,
 	err = pci_alloc_irq_vectors(dev, 1, 1, flags);
 	if (err < 0)
 		return err;
+	irq = pci_irq_vector(dev, 0);
+	if (irq < 0) {
+		err = irq;
+		goto fail_msi;
+	}
 
-	err = request_threaded_irq(dev->irq, irq_handler, NULL,
+	err = request_threaded_irq(irq, irq_handler, NULL,
 				   pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, ohci_driver_name,
 				   ohci);
 	if (err < 0) {
-		ohci_err(ohci, "failed to allocate interrupt %d\n", dev->irq);
+		ohci_err(ohci, "failed to allocate interrupt %d\n", irq);
 		goto fail_msi;
 	}
 
@@ -3779,7 +3784,7 @@ static int pci_probe(struct pci_dev *dev,
 	return 0;
 
  fail_irq:
-	free_irq(dev->irq, ohci);
+	free_irq(irq, ohci);
  fail_msi:
 	pci_free_irq_vectors(dev);
 
@@ -3789,6 +3794,7 @@ static int pci_probe(struct pci_dev *dev,
 static void pci_remove(struct pci_dev *dev)
 {
 	struct fw_ohci *ohci = pci_get_drvdata(dev);
+	int irq;
 
 	/*
 	 * If the removal is happening from the suspend state, LPS won't be
@@ -3808,7 +3814,9 @@ static void pci_remove(struct pci_dev *dev)
 
 	software_reset(ohci);
 
-	free_irq(dev->irq, ohci);
+	irq = pci_irq_vector(dev, 0);
+	if (irq >= 0)
+		free_irq(irq, ohci);
 	pci_free_irq_vectors(dev);
 
 	dev_notice(&dev->dev, "removing fw-ohci device\n");
-- 
2.43.0

Re: [PATCH 0/4] firewire: ohci: obsolete usage of legacy API for IRQ request and MSI

[Takashi Sakamoto]

On Sun, Mar 31, 2024 at 10:50:33PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> The pair of pci_enable_msi() and pci_disable_msi() is deprecated
> nowadays, while it is still used in 1394 OHCI driver in FireWire
> subsystem. This series of changes is to use modern API sets for the
> driver.
> 
> 
> Takashi Sakamoto (4):
>   Revert "firewire: ohci: use devres for requested IRQ"
>   firewire: ohci: replace request_irq() with request_threaded_irq()
>   firewire: ohci: obsolete usage of deprecated API for MSI
>   firewire: ohci: use pci_irq_vector() to retrieve allocated interrupt
>     line
> 
>  drivers/firewire/ohci.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)

Applied to for-next branch for v6.10.


Thanks

Takashi Sakamoto