[PATCH RFC 0/2] PCI: Restore original PCI_COMMAND value in pcim_release

[PATCH RFC 0/2] PCI: Restore original PCI_COMMAND value in pcim_release

[Heiner Kallweit]

Philipp's series refactors bigger parts of PCI devres. To reduce
complexity of the series it may be good to simplify the current
devres handling as much as possible first.

The patches proposed in my series allow to remove the MVI and INTX
related member from struct pci_devres.

If Philipp's patches allow to remove the region handling from
pcim_release(), and flag pinned is moved to struct pci_dev, then
I think we can completely get rid of struct pci_devres.

Heiner Kallweit (2):
  PCI: Restore original PCI_COMMAND value in pcim_release
  PCI: Remove MWI and INTX related members from struct pci_devres

 drivers/pci/devres.c | 15 ++-------------
 drivers/pci/pci.c    | 11 +----------
 drivers/pci/pci.h    |  3 ---
 drivers/pci/probe.c  |  3 +++
 include/linux/pci.h  |  1 +
 5 files changed, 7 insertions(+), 26 deletions(-)

-- 
2.44.0

[PATCH RFC 1/2] PCI: Restore original PCI_COMMAND value in pcim_release

[Heiner Kallweit]

Instead of the complex handling of INTX and MWI in PCi devres, let's
simply restore the original vale of PCI_COMMAND in pcim_release().

Only side effect I've seen so far is that the "enabling device" info
message is printed each time a driver is re-probed after a EPROBE_DEFER.
I propose to silence this message by changing it to debug level.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/pci/devres.c | 8 ++------
 drivers/pci/probe.c  | 3 +++
 include/linux/pci.h  | 1 +
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 2c562b9ea..7766f4df4 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -172,14 +172,10 @@ static void pcim_release(struct device *gendev, void *res)
 		if (this->region_mask & (1 << i))
 			pci_release_region(dev, i);
 
-	if (this->mwi)
-		pci_clear_mwi(dev);
-
-	if (this->restore_intx)
-		pci_intx(dev, this->orig_intx);
-
 	if (this->enabled && !this->pinned)
 		pci_disable_device(dev);
+
+	pci_write_config_word(dev, PCI_COMMAND, dev->pci_command);
 }
 
 /*
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 1325fbae2..60052c979 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2558,6 +2558,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 
 	pci_init_capabilities(dev);
 
+	/* Store original register value */
+	pci_read_config_word(dev, PCI_COMMAND, &dev->pci_command);
+
 	/*
 	 * Add the device to our list of discovered devices
 	 * and the bus list for fixup functions, etc.
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 16493426a..92c3c99c9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -352,6 +352,7 @@ struct pci_dev {
 	u8		rom_base_reg;	/* Config register controlling ROM */
 	u8		pin;		/* Interrupt pin this device uses */
 	u16		pcie_flags_reg;	/* Cached PCIe Capabilities Register */
+	u16		pci_command;	/* Restore original value in pci_disable_device */
 	unsigned long	*dma_alias_mask;/* Mask of enabled devfn aliases */
 
 	struct pci_driver *driver;	/* Driver bound to this device */
-- 
2.44.0

[PATCH RFC 2/2] PCI: Remove MWI and INTX related members from struct pci_devres

[Heiner Kallweit]

After patch 1 of the series the original value of register PCI_COMMAND
is restored in pcim_release. Therefore the code removed here isn't
needed any longer.

In a follow-up step pcim_set_mwi() could be removed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/pci/devres.c |  7 -------
 drivers/pci/pci.c    | 11 +----------
 drivers/pci/pci.h    |  3 ---
 3 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 7766f4df4..2661f7775 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -150,13 +150,6 @@ EXPORT_SYMBOL(devm_pci_remap_cfg_resource);
  */
 int pcim_set_mwi(struct pci_dev *dev)
 {
-	struct pci_devres *dr;
-
-	dr = find_pci_dr(dev);
-	if (!dr)
-		return -ENOMEM;
-
-	dr->mwi = 1;
 	return pci_set_mwi(dev);
 }
 EXPORT_SYMBOL(pcim_set_mwi);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e5f243dd4..2007edaaa 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4408,17 +4408,8 @@ void pci_intx(struct pci_dev *pdev, int enable)
 	else
 		new = pci_command | PCI_COMMAND_INTX_DISABLE;
 
-	if (new != pci_command) {
-		struct pci_devres *dr;
-
+	if (new != pci_command)
 		pci_write_config_word(pdev, PCI_COMMAND, new);
-
-		dr = find_pci_dr(pdev);
-		if (dr && !dr->restore_intx) {
-			dr->restore_intx = 1;
-			dr->orig_intx = !enable;
-		}
-	}
 }
 EXPORT_SYMBOL_GPL(pci_intx);
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 17fed1846..a1bb8191c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -825,9 +825,6 @@ static inline pci_power_t mid_pci_get_power_state(struct pci_dev *pdev)
 struct pci_devres {
 	unsigned int enabled:1;
 	unsigned int pinned:1;
-	unsigned int orig_intx:1;
-	unsigned int restore_intx:1;
-	unsigned int mwi:1;
 	u32 region_mask;
 };
 
-- 
2.44.0