Skip to contentSkip to conversation

Remove implicit devres from pci_intx()

PATCH In review Philipp Stanner 7 participants 25 messages Dec 9, 2024 – Jan 10, 2025

Subsystems (17)
Networking Network drivers KVM PCI +13 more subsystems
lore
Share & tools
lore

Philipp Stanner

author
#0 lore raw

v3 cover Remove implicit devres from pci_intx()

@Driver-Maintainers: Your driver might be touched by patch "Remove devres from pci_intx()". You might want to take a look.

Changes in v3:
- Add Thomas' RB.

Changes in v2:
- Drop pci_intx() deprecation patch.
- ata: Add RB from Sergey and Niklas.
- wifi: Add AB by Kalle.
- Drop INTx deprecation patch
- Drop ALSA / hda_intel patch because pci_intx() was removed from there in the meantime.

Changes since the RFC [1]:
- Add a patch deprecating pci{m}_intx(). (Heiner, Andy, Me)
- Add Acked-by's already given.
- Export pcim_intx() as a GPL function. (Alex)
- Drop patch for rts5280, since this driver will be removed quite soon. (Philipp Hortmann, Greg)
- Use early-return in pci_intx_unmanaged() and pci_intx(). (Andy)

Hi all,

this series removes a problematic feature from pci_intx(). That function sometimes implicitly uses devres for automatic cleanup. We should get rid of this implicit behavior.

To do so, a pci_intx() version that is always-managed, and one that is never-managed are provided. Then, all pci_intx() users are ported to the version they need. Afterwards, pci_intx() can be cleaned up and the users of the never-managed version be ported back to pci_intx().

This way we'd get this PCI API consistent again.

Patch "Remove devres from pci_intx()" obviously reverts the previous patches that made drivers use pci_intx_unmanaged(). But this way it's easier to review and approve. It also makes sure that each checked out commit should provide correct behavior, not just the entire series as a whole.

Merge plan for this is to enter through the PCI tree.

[1] https://lore.kernel.org/all/[email protected]/

Regards,
P.

Philipp Stanner (11):
PCI: Prepare removing devres from pci_intx() drivers/xen: Use never-managed version of pci_intx() net/ethernet: Use never-managed version of pci_intx() net/ntb: Use never-managed version of pci_intx()
misc: Use never-managed version of pci_intx() vfio/pci: Use never-managed version of pci_intx()
PCI: MSI: Use never-managed version of pci_intx()
ata: Use always-managed version of pci_intx()
wifi: qtnfmac: use always-managed version of pcim_intx()
HID: amd_sfh: Use always-managed version of pcim_intx() Remove devres from pci_intx()

 drivers/ata/ahci.c                            |  2 +-
 drivers/ata/ata_piix.c                        |  2 +-
 drivers/ata/pata_rdc.c                        |  2 +-
 drivers/ata/sata_sil24.c                      |  2 +-
 drivers/ata/sata_sis.c                        |  2 +-
 drivers/ata/sata_uli.c                        |  2 +-
 drivers/ata/sata_vsc.c                        |  2 +-
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c        |  4 ++--
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c |  2 +-
 .../wireless/quantenna/qtnfmac/pcie/pcie.c    |  2 +-
 drivers/pci/devres.c                          | 24 +++----------------
 drivers/pci/pci.c                             | 16 +++----------
 include/linux/pci.h                           |  1 +
 13 files changed, 18 insertions(+), 45 deletions(-)
Signature · 1 line
2.47.1
author
#1 lore raw

PATCH v3 1/11 PCI: Prepare removing devres from pci_intx()

pci_intx() is a hybrid function which sometimes performs devres operations, depending on whether pcim_enable_device() has been used to enable the pci_dev. This sometimes-managed nature of the function is problematic. Notably, it causes the function to allocate under some circumstances which makes it unusable from interrupt context.

To, ultimately, remove the hybrid nature from pci_intx(), it is first necessary to provide an always-managed and a never-managed version of that function. Then, all callers of pci_intx() can be ported to the version they need, depending whether they use pci_enable_device() or pcim_enable_device().

An always-managed function exists, namely pcim_intx(), for which __pcim_intx(), a never-managed version of pci_intx() has been implemented.

Make __pcim_intx() a public function under the name pci_intx_unmanaged(). Make pcim_intx() a public function.

Signed-off-by Philipp Stanner <[email protected]>
Reviewed-by Damien Le Moal <[email protected]>
---
 drivers/pci/devres.c | 24 +++---------------------
 drivers/pci/pci.c    | 29 +++++++++++++++++++++++++++++
 include/linux/pci.h  |  2 ++
 3 files changed, 34 insertions(+), 21 deletions(-)
drivers/pci/devres.c +3 -21
index 3b59a86a764b..3594eea37993 100644
@@ -411,31 +411,12 @@ static inline bool mask_contains_bar(int mask, int bar)
 	return mask & BIT(bar);
 }
 
-/*
- * This is a copy of pci_intx() used to bypass the problem of recursive
- * function calls due to the hybrid nature of pci_intx().
- */
-static void __pcim_intx(struct pci_dev *pdev, int enable)
-{
-	u16 pci_command, new;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-
-	if (enable)
-		new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
-	else
-		new = pci_command | PCI_COMMAND_INTX_DISABLE;
-
-	if (new != pci_command)
-		pci_write_config_word(pdev, PCI_COMMAND, new);
-}
-
 static void pcim_intx_restore(struct device *dev, void *data)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pcim_intx_devres *res = data;
 
-	__pcim_intx(pdev, res->orig_intx);
+	pci_intx_unmanaged(pdev, res->orig_intx);
 }
 
 static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev)

@@ -472,10 +453,11 @@ int pcim_intx(struct pci_dev *pdev, int enable)
 		return -ENOMEM;
 
 	res->orig_intx = !enable;
-	__pcim_intx(pdev, enable);
+	pci_intx_unmanaged(pdev, enable);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(pcim_intx);
 
 static void pcim_disable_device(void *pdev_raw)
 {
drivers/pci/pci.c +29 -0
index 0b29ec6e8e5e..30d17ec771fc 100644
@@ -4482,6 +4482,35 @@ void pci_disable_parity(struct pci_dev *dev)
 	}
 }
 
+/**
+ * pci_intx_unmanaged - enables/disables PCI INTx for device dev,
+ * unmanaged version
+ * @pdev: the PCI device to operate on
+ * @enable: boolean: whether to enable or disable PCI INTx
+ *
+ * Enables/disables PCI INTx for device @pdev
+ *
+ * This function behavios identically to pci_intx(), but is never managed with
+ * devres.
+ */
+void pci_intx_unmanaged(struct pci_dev *pdev, int enable)
+{
+	u16 pci_command, new;
+
+	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+
+	if (enable)
+		new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
+	else
+		new = pci_command | PCI_COMMAND_INTX_DISABLE;
+
+	if (new == pci_command)
+		return;
+
+	pci_write_config_word(pdev, PCI_COMMAND, new);
+}
+EXPORT_SYMBOL_GPL(pci_intx_unmanaged);
+
 /**
  * pci_intx - enables/disables PCI INTx for device dev
  * @pdev: the PCI device to operate on
include/linux/pci.h +2 -0
index db9b47ce3eef..b5eb8bda655d 100644
@@ -1350,6 +1350,7 @@ int __must_check pcim_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
 void pci_disable_parity(struct pci_dev *dev);
+void pci_intx_unmanaged(struct pci_dev *pdev, int enable);
 void pci_intx(struct pci_dev *dev, int enable);
 bool pci_check_and_mask_intx(struct pci_dev *dev);
 bool pci_check_and_unmask_intx(struct pci_dev *dev);

@@ -2297,6 +2298,7 @@ static inline void pci_fixup_device(enum pci_fixup_pass pass,
 				    struct pci_dev *dev) { }
 #endif
 
+int pcim_intx(struct pci_dev *pdev, int enabled);
 int pcim_request_all_regions(struct pci_dev *pdev, const char *name);
 void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen);
 void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar,
Signature · 1 line
2.47.1
author
#2 lore raw

PATCH v3 2/11 drivers/xen: Use never-managed version of pci_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

xen enables its PCI-Device with pci_enable_device(). Thus, it needs the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
Acked-by Juergen Gross <[email protected]>
---
 drivers/xen/xen-pciback/conf_space_header.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/xen/xen-pciback/conf_space_header.c +1 -1
index fc0332645966..8d26d64232e8 100644
@@ -106,7 +106,7 @@ static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
 
 	if (dev_data && dev_data->allow_interrupt_control &&
 	    ((cmd->val ^ value) & PCI_COMMAND_INTX_DISABLE))
-		pci_intx(dev, !(value & PCI_COMMAND_INTX_DISABLE));
+		pci_intx_unmanaged(dev, !(value & PCI_COMMAND_INTX_DISABLE));
 
 	cmd->val = value;
Signature · 1 line
2.47.1
author
#3 lore raw

PATCH v3 3/11 net/ethernet: Use never-managed version of pci_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

broadcom/bnx2x and brocade/bna enable their PCI-Device with pci_enable_device(). Thus, they need the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 2 +-
 drivers/net/ethernet/brocade/bna/bnad.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +1 -1
index 678829646cec..2ae63d6e6792 100644
@@ -1669,7 +1669,7 @@ static void bnx2x_igu_int_enable(struct bnx2x *bp)
 	REG_WR(bp, IGU_REG_PF_CONFIGURATION, val);
 
 	if (val & IGU_PF_CONF_INT_LINE_EN)
-		pci_intx(bp->pdev, true);
+		pci_intx_unmanaged(bp->pdev, true);
 
 	barrier();
drivers/net/ethernet/brocade/bna/bnad.c +1 -1
index ece6f3b48327..2b37462d406e 100644
@@ -2669,7 +2669,7 @@ bnad_enable_msix(struct bnad *bnad)
 		}
 	}
 
-	pci_intx(bnad->pcidev, 0);
+	pci_intx_unmanaged(bnad->pcidev, 0);
 
 	return;
Signature · 1 line
2.47.1
maintainer Acked-by on v3 3/11
#4 lore raw

Re: [PATCH v3 03/11] net/ethernet: Use never-managed version of pci_intx()

Quoted 10 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

broadcom/bnx2x and brocade/bna enable their PCI-Device with pci_enable_device(). Thus, they need the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
Acked-by Paolo Abeni <[email protected]>
#5 lore raw

Re: [PATCH v3 03/11] net/ethernet: Use never-managed version of pci_intx()

In reply to Philipp Stanner

[cc->to: Sudarsana, Manish, Rasesh]

Quoted 10 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

broadcom/bnx2x and brocade/bna enable their PCI-Device with pci_enable_device(). Thus, they need the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>

Hoping for an ack from Sudarsana, Manish, Rasesh.

Quoted 34 lines
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 2 +-
 drivers/net/ethernet/brocade/bna/bnad.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +1 -1
index 678829646cec..2ae63d6e6792 100644
@@ -1669,7 +1669,7 @@ static void bnx2x_igu_int_enable(struct bnx2x *bp)
 	REG_WR(bp, IGU_REG_PF_CONFIGURATION, val);
 
 	if (val & IGU_PF_CONF_INT_LINE_EN)
-		pci_intx(bp->pdev, true);
+		pci_intx_unmanaged(bp->pdev, true);
 
 	barrier();
drivers/net/ethernet/brocade/bna/bnad.c +1 -1
index ece6f3b48327..2b37462d406e 100644
@@ -2669,7 +2669,7 @@ bnad_enable_msix(struct bnad *bnad)
 		}
 	}
 
-	pci_intx(bnad->pcidev, 0);
+	pci_intx_unmanaged(bnad->pcidev, 0);
 
 	return;
Signature · 1 line
2.47.1
author
#6 lore raw

PATCH v3 4/11 net/ntb: Use never-managed version of pci_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

hw/amd and how/intel enable their PCI-Device with pci_enable_device(). Thus, they need the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
Acked-by Shyam Sundar S K <[email protected]> #for ntb_hw_amd.c
Acked-by Dave Jiang <[email protected]> # for ntb_hw_gen1.c
---
 drivers/ntb/hw/amd/ntb_hw_amd.c    | 4 ++--
 drivers/ntb/hw/intel/ntb_hw_gen1.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/ntb/hw/amd/ntb_hw_amd.c +2 -2
index d687e8c2cc78..b146f170e839 100644
@@ -791,7 +791,7 @@ static int ndev_init_isr(struct amd_ntb_dev *ndev,
 err_msi_enable:
 
 	/* Try to set up intx irq */
-	pci_intx(pdev, 1);
+	pci_intx_unmanaged(pdev, 1);
 
 	rc = request_irq(pdev->irq, ndev_irq_isr, IRQF_SHARED,
 			 "ndev_irq_isr", ndev);

@@ -831,7 +831,7 @@ static void ndev_deinit_isr(struct amd_ntb_dev *ndev)
 		if (pci_dev_msi_enabled(pdev))
 			pci_disable_msi(pdev);
 		else
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 	}
 }
drivers/ntb/hw/intel/ntb_hw_gen1.c +1 -1
index 079b8cd79785..9ad9d7fe227e 100644
@@ -445,7 +445,7 @@ int ndev_init_isr(struct intel_ntb_dev *ndev,
 
 	/* Try to set up intx irq */
 
-	pci_intx(pdev, 1);
+	pci_intx_unmanaged(pdev, 1);
 
 	rc = request_irq(pdev->irq, ndev_irq_isr, IRQF_SHARED,
 			 "ndev_irq_isr", ndev);
Signature · 1 line
2.47.1
author
#7 lore raw

PATCH v3 5/11 misc: Use never-managed version of pci_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

cardreader/rtsx_pcr.c and tifm_7xx1.c enable their PCI-Device with pci_enable_device(). Thus, they need the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
---
 drivers/misc/cardreader/rtsx_pcr.c | 2 +-
 drivers/misc/tifm_7xx1.c           | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/misc/cardreader/rtsx_pcr.c +1 -1
index be3d4e0e50cc..e25e6d560dd7 100644
@@ -1057,7 +1057,7 @@ static int rtsx_pci_acquire_irq(struct rtsx_pcr *pcr)
 	}
 
 	pcr->irq = pcr->pci->irq;
-	pci_intx(pcr->pci, !pcr->msi_en);
+	pci_intx_unmanaged(pcr->pci, !pcr->msi_en);
 
 	return 0;
 }
drivers/misc/tifm_7xx1.c +3 -3
index 1d54680d6ed2..5f9c7ccae8d2 100644
@@ -327,7 +327,7 @@ static int tifm_7xx1_probe(struct pci_dev *dev,
 		goto err_out;
 	}
 
-	pci_intx(dev, 1);
+	pci_intx_unmanaged(dev, 1);
 
 	fm = tifm_alloc_adapter(dev->device == PCI_DEVICE_ID_TI_XX21_XX11_FM
 				? 4 : 2, &dev->dev);

@@ -368,7 +368,7 @@ static int tifm_7xx1_probe(struct pci_dev *dev,
 err_out_free:
 	tifm_free_adapter(fm);
 err_out_int:
-	pci_intx(dev, 0);
+	pci_intx_unmanaged(dev, 0);
 	pci_release_regions(dev);
 err_out:
 	if (!pci_dev_busy)

@@ -392,7 +392,7 @@ static void tifm_7xx1_remove(struct pci_dev *dev)
 		tifm_7xx1_sock_power_off(tifm_7xx1_sock_addr(fm->addr, cnt));
 
 	iounmap(fm->addr);
-	pci_intx(dev, 0);
+	pci_intx_unmanaged(dev, 0);
 	pci_release_regions(dev);
 
 	pci_disable_device(dev);
Signature · 1 line
2.47.1
#8 lore raw

Re: [PATCH v3 05/11] misc: Use never-managed version of pci_intx()

[cc->to: Arnd, Greg, Alex]

Quoted 10 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

cardreader/rtsx_pcr.c and tifm_7xx1.c enable their PCI-Device with pci_enable_device(). Thus, they need the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>

Looking for ack from Arnd, Greg, Alex.

Quoted 52 lines
---
 drivers/misc/cardreader/rtsx_pcr.c | 2 +-
 drivers/misc/tifm_7xx1.c           | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/misc/cardreader/rtsx_pcr.c +1 -1
index be3d4e0e50cc..e25e6d560dd7 100644
@@ -1057,7 +1057,7 @@ static int rtsx_pci_acquire_irq(struct rtsx_pcr *pcr)
 	}
 
 	pcr->irq = pcr->pci->irq;
-	pci_intx(pcr->pci, !pcr->msi_en);
+	pci_intx_unmanaged(pcr->pci, !pcr->msi_en);
 
 	return 0;
 }
drivers/misc/tifm_7xx1.c +3 -3
index 1d54680d6ed2..5f9c7ccae8d2 100644
@@ -327,7 +327,7 @@ static int tifm_7xx1_probe(struct pci_dev *dev,
 		goto err_out;
 	}
 
-	pci_intx(dev, 1);
+	pci_intx_unmanaged(dev, 1);
 
 	fm = tifm_alloc_adapter(dev->device == PCI_DEVICE_ID_TI_XX21_XX11_FM
 				? 4 : 2, &dev->dev);

@@ -368,7 +368,7 @@ static int tifm_7xx1_probe(struct pci_dev *dev,
 err_out_free:
 	tifm_free_adapter(fm);
 err_out_int:
-	pci_intx(dev, 0);
+	pci_intx_unmanaged(dev, 0);
 	pci_release_regions(dev);
 err_out:
 	if (!pci_dev_busy)

@@ -392,7 +392,7 @@ static void tifm_7xx1_remove(struct pci_dev *dev)
 		tifm_7xx1_sock_power_off(tifm_7xx1_sock_addr(fm->addr, cnt));
 
 	iounmap(fm->addr);
-	pci_intx(dev, 0);
+	pci_intx_unmanaged(dev, 0);
 	pci_release_regions(dev);
 
 	pci_disable_device(dev);
Signature · 1 line
2.47.1
maintainer Acked-by
#9 lore raw

Re: [PATCH v3 05/11] misc: Use never-managed version of pci_intx()

Quoted 7 lines · Bjorn Helgaas

[cc->to: Arnd, Greg, Alex]

Quoted 4 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

Quoted 8 lines
Quoted 6 lines

cardreader/rtsx_pcr.c and tifm_7xx1.c enable their PCI-Device with pci_enable_device(). Thus, they need the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>

Looking for ack from Arnd, Greg, Alex.

Acked-by Greg Kroah-Hartman <[email protected]>
maintainer Acked-by on v3 5/11
#10 lore raw

Re: [PATCH v3 05/11] misc: Use never-managed version of pci_intx()

In reply to Philipp Stanner
Quoted 7 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

cardreader/rtsx_pcr.c and tifm_7xx1.c enable their PCI-Device with pci_enable_device(). Thus, they need the never-managed version.

Quoted 7 lines

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
---
 drivers/misc/cardreader/rtsx_pcr.c | 2 +-
 drivers/misc/tifm_7xx1.c           | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)
Acked-by Greg Kroah-Hartman <[email protected]>
author
#11 lore raw

PATCH v3 6/11 vfio/pci: Use never-managed version of pci_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

vfio enables its PCI-Device with pci_enable_device(). Thus, it needs the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
---
 drivers/vfio/pci/vfio_pci_core.c  |  2 +-
 drivers/vfio/pci/vfio_pci_intrs.c | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)
drivers/vfio/pci/vfio_pci_core.c +1 -1
index 1ab58da9f38a..90240c8d51aa 100644
@@ -498,7 +498,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
 		if (vfio_pci_nointx(pdev)) {
 			pci_info(pdev, "Masking broken INTx support\n");
 			vdev->nointx = true;
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 		} else
 			vdev->pci_2_3 = pci_intx_mask_supported(pdev);
 	}
drivers/vfio/pci/vfio_pci_intrs.c +5 -5
index 8382c5834335..40abb0b937a2 100644
@@ -118,7 +118,7 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
 	 */
 	if (unlikely(!is_intx(vdev))) {
 		if (vdev->pci_2_3)
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 		goto out_unlock;
 	}
 

@@ -132,7 +132,7 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
 		 * mask, not just when something is pending.
 		 */
 		if (vdev->pci_2_3)
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 		else
 			disable_irq_nosync(pdev->irq);
 

@@ -178,7 +178,7 @@ static int vfio_pci_intx_unmask_handler(void *opaque, void *data)
 	 */
 	if (unlikely(!is_intx(vdev))) {
 		if (vdev->pci_2_3)
-			pci_intx(pdev, 1);
+			pci_intx_unmanaged(pdev, 1);
 		goto out_unlock;
 	}
 

@@ -296,7 +296,7 @@ static int vfio_intx_enable(struct vfio_pci_core_device *vdev,
 	 */
 	ctx->masked = vdev->virq_disabled;
 	if (vdev->pci_2_3) {
-		pci_intx(pdev, !ctx->masked);
+		pci_intx_unmanaged(pdev, !ctx->masked);
 		irqflags = IRQF_SHARED;
 	} else {
 		irqflags = ctx->masked ? IRQF_NO_AUTOEN : 0;

@@ -569,7 +569,7 @@ static void vfio_msi_disable(struct vfio_pci_core_device *vdev, bool msix)
 	 * via their shutdown paths.  Restore for NoINTx devices.
 	 */
 	if (vdev->nointx)
-		pci_intx(pdev, 0);
+		pci_intx_unmanaged(pdev, 0);
 
 	vdev->irq_type = VFIO_PCI_NUM_IRQS;
 }
Signature · 1 line
2.47.1
#12 lore raw

Re: [PATCH v3 06/11] vfio/pci: Use never-managed version of pci_intx()

[cc->to: Alex W]

Quoted 10 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

vfio enables its PCI-Device with pci_enable_device(). Thus, it needs the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>

Not applied yet, pending ack from Alex.

Quoted 70 lines
---
 drivers/vfio/pci/vfio_pci_core.c  |  2 +-
 drivers/vfio/pci/vfio_pci_intrs.c | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)
drivers/vfio/pci/vfio_pci_core.c +1 -1
index 1ab58da9f38a..90240c8d51aa 100644
@@ -498,7 +498,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
 		if (vfio_pci_nointx(pdev)) {
 			pci_info(pdev, "Masking broken INTx support\n");
 			vdev->nointx = true;
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 		} else
 			vdev->pci_2_3 = pci_intx_mask_supported(pdev);
 	}
drivers/vfio/pci/vfio_pci_intrs.c +5 -5
index 8382c5834335..40abb0b937a2 100644
@@ -118,7 +118,7 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
 	 */
 	if (unlikely(!is_intx(vdev))) {
 		if (vdev->pci_2_3)
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 		goto out_unlock;
 	}
 

@@ -132,7 +132,7 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
 		 * mask, not just when something is pending.
 		 */
 		if (vdev->pci_2_3)
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 		else
 			disable_irq_nosync(pdev->irq);
 

@@ -178,7 +178,7 @@ static int vfio_pci_intx_unmask_handler(void *opaque, void *data)
 	 */
 	if (unlikely(!is_intx(vdev))) {
 		if (vdev->pci_2_3)
-			pci_intx(pdev, 1);
+			pci_intx_unmanaged(pdev, 1);
 		goto out_unlock;
 	}
 

@@ -296,7 +296,7 @@ static int vfio_intx_enable(struct vfio_pci_core_device *vdev,
 	 */
 	ctx->masked = vdev->virq_disabled;
 	if (vdev->pci_2_3) {
-		pci_intx(pdev, !ctx->masked);
+		pci_intx_unmanaged(pdev, !ctx->masked);
 		irqflags = IRQF_SHARED;
 	} else {
 		irqflags = ctx->masked ? IRQF_NO_AUTOEN : 0;

@@ -569,7 +569,7 @@ static void vfio_msi_disable(struct vfio_pci_core_device *vdev, bool msix)
 	 * via their shutdown paths.  Restore for NoINTx devices.
 	 */
 	if (vdev->nointx)
-		pci_intx(pdev, 0);
+		pci_intx_unmanaged(pdev, 0);
 
 	vdev->irq_type = VFIO_PCI_NUM_IRQS;
 }
Signature · 1 line
2.47.1
Acked-by
#13 lore raw

Re: [PATCH v3 06/11] vfio/pci: Use never-managed version of pci_intx()

Quoted 7 lines · On Thu, 12 Dec 2024 13:21:30 -0600 Bjorn Helgaas <[email protected]> wrote:

[cc->to: Alex W]

Quoted 4 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

Quoted 8 lines
Quoted 6 lines

vfio enables its PCI-Device with pci_enable_device(). Thus, it needs the never-managed version.

Replace pci_intx() with pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>

Not applied yet, pending ack from Alex.

Acked-by Alex Williamson <[email protected]>
Quoted 72 lines
Quoted 71 lines
---
 drivers/vfio/pci/vfio_pci_core.c  |  2 +-
 drivers/vfio/pci/vfio_pci_intrs.c | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)
drivers/vfio/pci/vfio_pci_core.c +1 -1
index 1ab58da9f38a..90240c8d51aa 100644
@@ -498,7 +498,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
 		if (vfio_pci_nointx(pdev)) {
 			pci_info(pdev, "Masking broken INTx support\n");
 			vdev->nointx = true;
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 		} else
 			vdev->pci_2_3 = pci_intx_mask_supported(pdev);
 	}
drivers/vfio/pci/vfio_pci_intrs.c +5 -5
index 8382c5834335..40abb0b937a2 100644
@@ -118,7 +118,7 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
 	 */
 	if (unlikely(!is_intx(vdev))) {
 		if (vdev->pci_2_3)
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 		goto out_unlock;
 	}
 

@@ -132,7 +132,7 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
 		 * mask, not just when something is pending.
 		 */
 		if (vdev->pci_2_3)
-			pci_intx(pdev, 0);
+			pci_intx_unmanaged(pdev, 0);
 		else
 			disable_irq_nosync(pdev->irq);
 

@@ -178,7 +178,7 @@ static int vfio_pci_intx_unmask_handler(void *opaque, void *data)
 	 */
 	if (unlikely(!is_intx(vdev))) {
 		if (vdev->pci_2_3)
-			pci_intx(pdev, 1);
+			pci_intx_unmanaged(pdev, 1);
 		goto out_unlock;
 	}
 

@@ -296,7 +296,7 @@ static int vfio_intx_enable(struct vfio_pci_core_device *vdev,
 	 */
 	ctx->masked = vdev->virq_disabled;
 	if (vdev->pci_2_3) {
-		pci_intx(pdev, !ctx->masked);
+		pci_intx_unmanaged(pdev, !ctx->masked);
 		irqflags = IRQF_SHARED;
 	} else {
 		irqflags = ctx->masked ? IRQF_NO_AUTOEN : 0;

@@ -569,7 +569,7 @@ static void vfio_msi_disable(struct vfio_pci_core_device *vdev, bool msix)
 	 * via their shutdown paths.  Restore for NoINTx devices.
 	 */
 	if (vdev->nointx)
-		pci_intx(pdev, 0);
+		pci_intx_unmanaged(pdev, 0);
 
 	vdev->irq_type = VFIO_PCI_NUM_IRQS;
 }
Signature · 1 line
2.47.1
author
#14 lore raw

PATCH v3 7/11 PCI: MSI: Use never-managed version of pci_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

MSI sets up its own separate devres callback implicitly in pcim_setup_msi_release(). This callback ultimately uses pci_intx(), which is problematic since the callback of course runs on driver-detach.

That problem has last been described here:
https://lore.kernel.org/all/[email protected]/

Replace the call to pci_intx() with one to the never-managed version pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
---
 drivers/pci/msi/api.c | 2 +-
 drivers/pci/msi/msi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/pci/msi/api.c +1 -1
index b956ce591f96..c95e2e7dc9ab 100644
@@ -289,7 +289,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 			 */
 			if (affd)
 				irq_create_affinity_masks(1, affd);
-			pci_intx(dev, 1);
+			pci_intx_unmanaged(dev, 1);
 			return 1;
 		}
 	}
drivers/pci/msi/msi.c +1 -1
index 3a45879d85db..53f13b09db50 100644
@@ -268,7 +268,7 @@ EXPORT_SYMBOL_GPL(pci_write_msi_msg);
 static void pci_intx_for_msi(struct pci_dev *dev, int enable)
 {
 	if (!(dev->dev_flags & PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG))
-		pci_intx(dev, enable);
+		pci_intx_unmanaged(dev, enable);
 }
 
 static void pci_msi_set_enable(struct pci_dev *dev, int enable)
Signature · 1 line
2.47.1
author
#15 lore raw

PATCH v3 8/11 ata: Use always-managed version of pci_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

All users in ata enable their PCI-Device with pcim_enable_device(). Thus, they need the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>
Reviewed-by Sergey Shtylyov <[email protected]>
Acked-by Niklas Cassel <[email protected]>
---
 drivers/ata/ahci.c       | 2 +-
 drivers/ata/ata_piix.c   | 2 +-
 drivers/ata/pata_rdc.c   | 2 +-
 drivers/ata/sata_sil24.c | 2 +-
 drivers/ata/sata_sis.c   | 2 +-
 drivers/ata/sata_uli.c   | 2 +-
 drivers/ata/sata_vsc.c   | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)
drivers/ata/ahci.c +1 -1
index 8d27c567be1c..f813dbdc2346 100644
@@ -1987,7 +1987,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	if (ahci_init_msi(pdev, n_ports, hpriv) < 0) {
 		/* legacy intx interrupts */
-		pci_intx(pdev, 1);
+		pcim_intx(pdev, 1);
 	}
 	hpriv->irq = pci_irq_vector(pdev, 0);
drivers/ata/ata_piix.c +1 -1
index 093b940bc953..d441246fa357 100644
@@ -1725,7 +1725,7 @@ static int piix_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * message-signalled interrupts currently).
 	 */
 	if (port_flags & PIIX_FLAG_CHECKINTR)
-		pci_intx(pdev, 1);
+		pcim_intx(pdev, 1);
 
 	if (piix_check_450nx_errata(pdev)) {
 		/* This writes into the master table but it does not
drivers/ata/pata_rdc.c +1 -1
index 0a9689862f71..09792aac7f9d 100644
@@ -340,7 +340,7 @@ static int rdc_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return rc;
 	host->private_data = hpriv;
 
-	pci_intx(pdev, 1);
+	pcim_intx(pdev, 1);
 
 	host->flags |= ATA_HOST_PARALLEL_SCAN;
drivers/ata/sata_sil24.c +1 -1
index 72c03cbdaff4..b771ebd41252 100644
@@ -1317,7 +1317,7 @@ static int sil24_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	if (sata_sil24_msi && !pci_enable_msi(pdev)) {
 		dev_info(&pdev->dev, "Using MSI\n");
-		pci_intx(pdev, 0);
+		pcim_intx(pdev, 0);
 	}
 
 	pci_set_master(pdev);
drivers/ata/sata_sis.c +1 -1
index ef8724986de3..b8b6d9eff3b8 100644
@@ -290,7 +290,7 @@ static int sis_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 
 	pci_set_master(pdev);
-	pci_intx(pdev, 1);
+	pcim_intx(pdev, 1);
 	return ata_host_activate(host, pdev->irq, ata_bmdma_interrupt,
 				 IRQF_SHARED, &sis_sht);
 }
drivers/ata/sata_uli.c +1 -1
index 60ea45926cd1..52894ff49dcb 100644
@@ -221,7 +221,7 @@ static int uli_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 
 	pci_set_master(pdev);
-	pci_intx(pdev, 1);
+	pcim_intx(pdev, 1);
 	return ata_host_activate(host, pdev->irq, ata_bmdma_interrupt,
 				 IRQF_SHARED, &uli_sht);
 }
drivers/ata/sata_vsc.c +1 -1
index d39b87537168..a53a2dfc1e17 100644
@@ -384,7 +384,7 @@ static int vsc_sata_init_one(struct pci_dev *pdev,
 		pci_write_config_byte(pdev, PCI_CACHE_LINE_SIZE, 0x80);
 
 	if (pci_enable_msi(pdev) == 0)
-		pci_intx(pdev, 0);
+		pcim_intx(pdev, 0);
 
 	/*
 	 * Config offset 0x98 is "Extended Control and Status Register 0"
Signature · 1 line
2.47.1
author
#16 lore raw

PATCH v3 9/11 wifi: qtnfmac: use always-managed version of pcim_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

qtnfmac enables its PCI-Device with pcim_enable_device(). Thus, it needs the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>
Acked-by Kalle Valo <[email protected]>
---
 drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c +1 -1
index f66eb43094d4..3adcfac2886f 100644
@@ -204,7 +204,7 @@ static void qtnf_pcie_init_irq(struct qtnf_pcie_bus_priv *priv, bool use_msi)
 
 	if (!priv->msi_enabled) {
 		pr_warn("legacy PCIE interrupts enabled\n");
-		pci_intx(pdev, 1);
+		pcim_intx(pdev, 1);
 	}
 }
Signature · 1 line
2.47.1
#17 lore raw

Re: [PATCH v3 09/11] wifi: qtnfmac: use always-managed version of pcim_intx()

[cc->to: Igor]

Quoted 11 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

qtnfmac enables its PCI-Device with pcim_enable_device(). Thus, it needs the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>
Acked-by Kalle Valo <[email protected]>

Hoping for an ack from Igor, too.

Quoted 20 lines
---
 drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c +1 -1
index f66eb43094d4..3adcfac2886f 100644
@@ -204,7 +204,7 @@ static void qtnf_pcie_init_irq(struct qtnf_pcie_bus_priv *priv, bool use_msi)
 
 	if (!priv->msi_enabled) {
 		pr_warn("legacy PCIE interrupts enabled\n");
-		pci_intx(pdev, 1);
+		pcim_intx(pdev, 1);
 	}
 }
Signature · 1 line
2.47.1
#18 lore raw

Re: [PATCH v3 09/11] wifi: qtnfmac: use always-managed version of pcim_intx()

Quoted 10 lines · Bjorn Helgaas

[cc->to: Igor]

Quoted 7 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

qtnfmac enables its PCI-Device with pcim_enable_device(). Thus, it needs the always-managed version.

Quoted 6 lines
Quoted 4 lines

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>
Acked-by Kalle Valo <[email protected]>

Hoping for an ack from Igor, too.

Igor hasn't been around for a while so I'm not expecting see an ack from him, I think the whole qtnfmac driver should be removed in the future. Feel free to take the patch as is.

Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
#19 lore raw

Re: [PATCH v3 09/11] wifi: qtnfmac: use always-managed version of pcim_intx()

[+cc personal address for Igor]

Quoted 14 lines · Kalle Valo
Quoted 12 lines · Bjorn Helgaas

[cc->to: Igor]

Quoted 9 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

qtnfmac enables its PCI-Device with pcim_enable_device(). Thus, it needs the always-managed version.

Replace pci_intx() with pcim_intx().

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Signed-off-by Philipp Stanner <[email protected]>
Acked-by Kalle Valo <[email protected]>

Hoping for an ack from Igor, too.

Igor hasn't been around for a while so I'm not expecting see an ack from him, I think the whole qtnfmac driver should be removed in the future. Feel free to take the patch as is.

Thanks, Kalle, will do.

author
#20 lore raw

PATCH v3 10/11 HID: amd_sfh: Use always-managed version of pcim_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. To remove this hybrid nature from pci_intx(), it is necessary to port users to either an always-managed or a never-managed version.

All users of amd_mp2_pci_remove(), where pci_intx() is used, call pcim_enable_device(), which is why the driver needs the always-managed version.

Replace pci_intx() with pcim_intx().

Signed-off-by Philipp Stanner <[email protected]>
Acked-by Basavaraj Natikar <[email protected]>
---
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c        | 4 ++--
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/hid/amd-sfh-hid/amd_sfh_pcie.c +2 -2
index 0c28ca349bcd..48cfd0c58241 100644
@@ -122,7 +122,7 @@ int amd_sfh_irq_init_v2(struct amd_mp2_dev *privdata)
 {
 	int rc;
 
-	pci_intx(privdata->pdev, true);
+	pcim_intx(privdata->pdev, true);
 
 	rc = devm_request_irq(&privdata->pdev->dev, privdata->pdev->irq,
 			      amd_sfh_irq_handler, 0, DRIVER_NAME, privdata);

@@ -248,7 +248,7 @@ static void amd_mp2_pci_remove(void *privdata)
 	struct amd_mp2_dev *mp2 = privdata;
 	amd_sfh_hid_client_deinit(privdata);
 	mp2->mp2_ops->stop_all(mp2);
-	pci_intx(mp2->pdev, false);
+	pcim_intx(mp2->pdev, false);
 	amd_sfh_clear_intr(mp2);
 }
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c +1 -1
index db36d87d5634..ec9feb8e023b 100644
@@ -289,7 +289,7 @@ static void amd_mp2_pci_remove(void *privdata)
 	sfh_deinit_emp2();
 	amd_sfh_hid_client_deinit(privdata);
 	mp2->mp2_ops->stop_all(mp2);
-	pci_intx(mp2->pdev, false);
+	pcim_intx(mp2->pdev, false);
 	amd_sfh_clear_intr(mp2);
 }
Signature · 1 line
2.47.1
author
#21 lore raw

PATCH v3 11/11 Remove devres from pci_intx()

In reply to Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. This hybrid nature is undesirable.

Since all users of pci_intx() have by now been ported either to always-managed pcim_intx() or never-managed pci_intx_unmanaged(), the devres functionality can be removed from pci_intx().

Consequently, pci_intx_unmanaged() is now redundant, because pci_intx() itself is now unmanaged.

Remove the devres functionality from pci_intx(). Have all users of pci_intx_unmanaged() call pci_intx(). Remove pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
---
 drivers/misc/cardreader/rtsx_pcr.c            |  2 +-
 drivers/misc/tifm_7xx1.c                      |  6 +--
 .../net/ethernet/broadcom/bnx2x/bnx2x_main.c  |  2 +-
 drivers/net/ethernet/brocade/bna/bnad.c       |  2 +-
 drivers/ntb/hw/amd/ntb_hw_amd.c               |  4 +-
 drivers/ntb/hw/intel/ntb_hw_gen1.c            |  2 +-
 drivers/pci/devres.c                          |  4 +-
 drivers/pci/msi/api.c                         |  2 +-
 drivers/pci/msi/msi.c                         |  2 +-
 drivers/pci/pci.c                             | 43 +------------------
 drivers/vfio/pci/vfio_pci_core.c              |  2 +-
 drivers/vfio/pci/vfio_pci_intrs.c             | 10 ++---
 drivers/xen/xen-pciback/conf_space_header.c   |  2 +-
 include/linux/pci.h                           |  1 -
 14 files changed, 22 insertions(+), 62 deletions(-)
drivers/misc/cardreader/rtsx_pcr.c +1 -1
index e25e6d560dd7..be3d4e0e50cc 100644
@@ -1057,7 +1057,7 @@ static int rtsx_pci_acquire_irq(struct rtsx_pcr *pcr)
 	}
 
 	pcr->irq = pcr->pci->irq;
-	pci_intx_unmanaged(pcr->pci, !pcr->msi_en);
+	pci_intx(pcr->pci, !pcr->msi_en);
 
 	return 0;
 }
drivers/misc/tifm_7xx1.c +3 -3
index 5f9c7ccae8d2..1d54680d6ed2 100644
@@ -327,7 +327,7 @@ static int tifm_7xx1_probe(struct pci_dev *dev,
 		goto err_out;
 	}
 
-	pci_intx_unmanaged(dev, 1);
+	pci_intx(dev, 1);
 
 	fm = tifm_alloc_adapter(dev->device == PCI_DEVICE_ID_TI_XX21_XX11_FM
 				? 4 : 2, &dev->dev);

@@ -368,7 +368,7 @@ static int tifm_7xx1_probe(struct pci_dev *dev,
 err_out_free:
 	tifm_free_adapter(fm);
 err_out_int:
-	pci_intx_unmanaged(dev, 0);
+	pci_intx(dev, 0);
 	pci_release_regions(dev);
 err_out:
 	if (!pci_dev_busy)

@@ -392,7 +392,7 @@ static void tifm_7xx1_remove(struct pci_dev *dev)
 		tifm_7xx1_sock_power_off(tifm_7xx1_sock_addr(fm->addr, cnt));
 
 	iounmap(fm->addr);
-	pci_intx_unmanaged(dev, 0);
+	pci_intx(dev, 0);
 	pci_release_regions(dev);
 
 	pci_disable_device(dev);
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +1 -1
index 2ae63d6e6792..678829646cec 100644
@@ -1669,7 +1669,7 @@ static void bnx2x_igu_int_enable(struct bnx2x *bp)
 	REG_WR(bp, IGU_REG_PF_CONFIGURATION, val);
 
 	if (val & IGU_PF_CONF_INT_LINE_EN)
-		pci_intx_unmanaged(bp->pdev, true);
+		pci_intx(bp->pdev, true);
 
 	barrier();
drivers/net/ethernet/brocade/bna/bnad.c +1 -1
index 2b37462d406e..ece6f3b48327 100644
@@ -2669,7 +2669,7 @@ bnad_enable_msix(struct bnad *bnad)
 		}
 	}
 
-	pci_intx_unmanaged(bnad->pcidev, 0);
+	pci_intx(bnad->pcidev, 0);
 
 	return;
drivers/ntb/hw/amd/ntb_hw_amd.c +2 -2
index b146f170e839..d687e8c2cc78 100644
@@ -791,7 +791,7 @@ static int ndev_init_isr(struct amd_ntb_dev *ndev,
 err_msi_enable:
 
 	/* Try to set up intx irq */
-	pci_intx_unmanaged(pdev, 1);
+	pci_intx(pdev, 1);
 
 	rc = request_irq(pdev->irq, ndev_irq_isr, IRQF_SHARED,
 			 "ndev_irq_isr", ndev);

@@ -831,7 +831,7 @@ static void ndev_deinit_isr(struct amd_ntb_dev *ndev)
 		if (pci_dev_msi_enabled(pdev))
 			pci_disable_msi(pdev);
 		else
-			pci_intx_unmanaged(pdev, 0);
+			pci_intx(pdev, 0);
 	}
 }
drivers/ntb/hw/intel/ntb_hw_gen1.c +1 -1
index 9ad9d7fe227e..079b8cd79785 100644
@@ -445,7 +445,7 @@ int ndev_init_isr(struct intel_ntb_dev *ndev,
 
 	/* Try to set up intx irq */
 
-	pci_intx_unmanaged(pdev, 1);
+	pci_intx(pdev, 1);
 
 	rc = request_irq(pdev->irq, ndev_irq_isr, IRQF_SHARED,
 			 "ndev_irq_isr", ndev);
drivers/pci/devres.c +2 -2
index 3594eea37993..cc3195134721 100644
@@ -416,7 +416,7 @@ static void pcim_intx_restore(struct device *dev, void *data)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pcim_intx_devres *res = data;
 
-	pci_intx_unmanaged(pdev, res->orig_intx);
+	pci_intx(pdev, res->orig_intx);
 }
 
 static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev)

@@ -453,7 +453,7 @@ int pcim_intx(struct pci_dev *pdev, int enable)
 		return -ENOMEM;
 
 	res->orig_intx = !enable;
-	pci_intx_unmanaged(pdev, enable);
+	pci_intx(pdev, enable);
 
 	return 0;
 }
drivers/pci/msi/api.c +1 -1
index c95e2e7dc9ab..b956ce591f96 100644
@@ -289,7 +289,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 			 */
 			if (affd)
 				irq_create_affinity_masks(1, affd);
-			pci_intx_unmanaged(dev, 1);
+			pci_intx(dev, 1);
 			return 1;
 		}
 	}
drivers/pci/msi/msi.c +1 -1
index 53f13b09db50..3a45879d85db 100644
@@ -268,7 +268,7 @@ EXPORT_SYMBOL_GPL(pci_write_msi_msg);
 static void pci_intx_for_msi(struct pci_dev *dev, int enable)
 {
 	if (!(dev->dev_flags & PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG))
-		pci_intx_unmanaged(dev, enable);
+		pci_intx(dev, enable);
 }
 
 static void pci_msi_set_enable(struct pci_dev *dev, int enable)
drivers/pci/pci.c +2 -41

60 lines not on this page · raw message

drivers/vfio/pci/vfio_pci_core.c +1 -1
index 90240c8d51aa..1ab58da9f38a 100644
@@ -498,7 +498,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
 		if (vfio_pci_nointx(pdev)) {
 			pci_info(pdev, "Masking broken INTx support\n");
 			vdev->nointx = true;
-			pci_intx_unmanaged(pdev, 0);
+			pci_intx(pdev, 0);
 		} else
 			vdev->pci_2_3 = pci_intx_mask_supported(pdev);
 	}
drivers/vfio/pci/vfio_pci_intrs.c +5 -5
index 40abb0b937a2..8382c5834335 100644
@@ -118,7 +118,7 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
 	 */
 	if (unlikely(!is_intx(vdev))) {
 		if (vdev->pci_2_3)
-			pci_intx_unmanaged(pdev, 0);
+			pci_intx(pdev, 0);
 		goto out_unlock;
 	}
 

@@ -132,7 +132,7 @@ static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
 		 * mask, not just when something is pending.
 		 */
 		if (vdev->pci_2_3)
-			pci_intx_unmanaged(pdev, 0);
+			pci_intx(pdev, 0);
 		else
 			disable_irq_nosync(pdev->irq);
 

@@ -178,7 +178,7 @@ static int vfio_pci_intx_unmask_handler(void *opaque, void *data)
 	 */
 	if (unlikely(!is_intx(vdev))) {
 		if (vdev->pci_2_3)
-			pci_intx_unmanaged(pdev, 1);
+			pci_intx(pdev, 1);
 		goto out_unlock;
 	}
 

@@ -296,7 +296,7 @@ static int vfio_intx_enable(struct vfio_pci_core_device *vdev,
 	 */
 	ctx->masked = vdev->virq_disabled;
 	if (vdev->pci_2_3) {
-		pci_intx_unmanaged(pdev, !ctx->masked);
+		pci_intx(pdev, !ctx->masked);
 		irqflags = IRQF_SHARED;
 	} else {
 		irqflags = ctx->masked ? IRQF_NO_AUTOEN : 0;

@@ -569,7 +569,7 @@ static void vfio_msi_disable(struct vfio_pci_core_device *vdev, bool msix)
 	 * via their shutdown paths.  Restore for NoINTx devices.
 	 */
 	if (vdev->nointx)
-		pci_intx_unmanaged(pdev, 0);
+		pci_intx(pdev, 0);
 
 	vdev->irq_type = VFIO_PCI_NUM_IRQS;
 }
drivers/xen/xen-pciback/conf_space_header.c +1 -1
index 8d26d64232e8..fc0332645966 100644
@@ -106,7 +106,7 @@ static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
 
 	if (dev_data && dev_data->allow_interrupt_control &&
 	    ((cmd->val ^ value) & PCI_COMMAND_INTX_DISABLE))
-		pci_intx_unmanaged(dev, !(value & PCI_COMMAND_INTX_DISABLE));
+		pci_intx(dev, !(value & PCI_COMMAND_INTX_DISABLE));
 
 	cmd->val = value;
include/linux/pci.h +0 -1
index b5eb8bda655d..f05903dd7695 100644
@@ -1350,7 +1350,6 @@ int __must_check pcim_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
 void pci_disable_parity(struct pci_dev *dev);
-void pci_intx_unmanaged(struct pci_dev *pdev, int enable);
 void pci_intx(struct pci_dev *dev, int enable);
 bool pci_check_and_mask_intx(struct pci_dev *dev);
 bool pci_check_and_unmask_intx(struct pci_dev *dev);
Signature · 1 line
2.47.1
maintainer Acked-by on v3 11/11
#22 lore raw

Re: [PATCH v3 11/11] Remove devres from pci_intx()

Quoted 14 lines · Philipp Stanner

pci_intx() is a hybrid function which can sometimes be managed through devres. This hybrid nature is undesirable.

Since all users of pci_intx() have by now been ported either to always-managed pcim_intx() or never-managed pci_intx_unmanaged(), the devres functionality can be removed from pci_intx().

Consequently, pci_intx_unmanaged() is now redundant, because pci_intx() itself is now unmanaged.

Remove the devres functionality from pci_intx(). Have all users of pci_intx_unmanaged() call pci_intx(). Remove pci_intx_unmanaged().

Signed-off-by Philipp Stanner <[email protected]>
Quoted 16 lines
---
 drivers/misc/cardreader/rtsx_pcr.c            |  2 +-
 drivers/misc/tifm_7xx1.c                      |  6 +--
 .../net/ethernet/broadcom/bnx2x/bnx2x_main.c  |  2 +-
 drivers/net/ethernet/brocade/bna/bnad.c       |  2 +-
 drivers/ntb/hw/amd/ntb_hw_amd.c               |  4 +-
 drivers/ntb/hw/intel/ntb_hw_gen1.c            |  2 +-
 drivers/pci/devres.c                          |  4 +-
 drivers/pci/msi/api.c                         |  2 +-
 drivers/pci/msi/msi.c                         |  2 +-
 drivers/pci/pci.c                             | 43 +------------------
 drivers/vfio/pci/vfio_pci_core.c              |  2 +-
 drivers/vfio/pci/vfio_pci_intrs.c             | 10 ++---
 drivers/xen/xen-pciback/conf_space_header.c   |  2 +-
 include/linux/pci.h                           |  1 -
 14 files changed, 22 insertions(+), 62 deletions(-)

For the net bits:

Acked-by Paolo Abeni <[email protected]>
#23 lore raw
In reply to Philipp Stanner
Quoted 41 lines · Philipp Stanner

@Driver-Maintainers: Your driver might be touched by patch "Remove devres from pci_intx()". You might want to take a look.

Changes in v3:
- Add Thomas' RB.

Changes in v2:
- Drop pci_intx() deprecation patch.
- ata: Add RB from Sergey and Niklas.
- wifi: Add AB by Kalle.
- Drop INTx deprecation patch
- Drop ALSA / hda_intel patch because pci_intx() was removed from there in the meantime.

Changes since the RFC [1]:
- Add a patch deprecating pci{m}_intx(). (Heiner, Andy, Me)
- Add Acked-by's already given.
- Export pcim_intx() as a GPL function. (Alex)
- Drop patch for rts5280, since this driver will be removed quite soon. (Philipp Hortmann, Greg)
- Use early-return in pci_intx_unmanaged() and pci_intx(). (Andy)

Hi all,

this series removes a problematic feature from pci_intx(). That function sometimes implicitly uses devres for automatic cleanup. We should get rid of this implicit behavior.

To do so, a pci_intx() version that is always-managed, and one that is never-managed are provided. Then, all pci_intx() users are ported to the version they need. Afterwards, pci_intx() can be cleaned up and the users of the never-managed version be ported back to pci_intx().

This way we'd get this PCI API consistent again.

Patch "Remove devres from pci_intx()" obviously reverts the previous patches that made drivers use pci_intx_unmanaged(). But this way it's easier to review and approve. It also makes sure that each checked out commit should provide correct behavior, not just the entire series as a whole.

Quoted 3 lines

Merge plan for this is to enter through the PCI tree.

[1] https://lore.kernel.org/all/[email protected]/

Applied the ones with maintainer acks to pci/devres for v6.14, thanks!

I'll poke the ones that haven't been acked yet.

Quoted 31 lines

Philipp Stanner (11):
PCI: Prepare removing devres from pci_intx() drivers/xen: Use never-managed version of pci_intx() net/ethernet: Use never-managed version of pci_intx() net/ntb: Use never-managed version of pci_intx()
misc: Use never-managed version of pci_intx() vfio/pci: Use never-managed version of pci_intx()
PCI: MSI: Use never-managed version of pci_intx()
ata: Use always-managed version of pci_intx()
wifi: qtnfmac: use always-managed version of pcim_intx()
HID: amd_sfh: Use always-managed version of pcim_intx() Remove devres from pci_intx()

 drivers/ata/ahci.c                            |  2 +-
 drivers/ata/ata_piix.c                        |  2 +-
 drivers/ata/pata_rdc.c                        |  2 +-
 drivers/ata/sata_sil24.c                      |  2 +-
 drivers/ata/sata_sis.c                        |  2 +-
 drivers/ata/sata_uli.c                        |  2 +-
 drivers/ata/sata_vsc.c                        |  2 +-
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c        |  4 ++--
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c |  2 +-
 .../wireless/quantenna/qtnfmac/pcie/pcie.c    |  2 +-
 drivers/pci/devres.c                          | 24 +++----------------
 drivers/pci/pci.c                             | 16 +++----------
 include/linux/pci.h                           |  1 +
 13 files changed, 18 insertions(+), 45 deletions(-)
Signature · 1 line
2.47.1