diff mbox series

[09/10] pci: devres: remove legacy pcim_release()

Message ID 20240115144655.32046-11-pstanner@redhat.com (mailing list archive)
State Superseded
Delegated to: Bjorn Helgaas
Headers show
Series Make PCI's devres API more consistent | expand

Commit Message

Philipp Stanner Jan. 15, 2024, 2:46 p.m. UTC
Thanks to preceding cleanup steps, pcim_release() is now not needed
anymore and can be replaced by pcim_disable_device(), which is the exact
counterpart to pcim_enable_device().
This permits removing further parts of the old devres API.

Replace pcim_release() with pcim_disable_device().
Remove the now surplus get_dr() function.

Signed-off-by: Philipp Stanner <pstanner@redhat.com>
---
 drivers/pci/devres.c | 38 ++++++++------------------------------
 1 file changed, 8 insertions(+), 30 deletions(-)

Comments

Andy Shevchenko Jan. 16, 2024, 9:40 p.m. UTC | #1
Mon, Jan 15, 2024 at 03:46:20PM +0100, Philipp Stanner kirjoitti:
> Thanks to preceding cleanup steps, pcim_release() is now not needed
> anymore and can be replaced by pcim_disable_device(), which is the exact
> counterpart to pcim_enable_device().
> This permits removing further parts of the old devres API.
> 
> Replace pcim_release() with pcim_disable_device().
> Remove the now surplus get_dr() function.

...

> +	devm_add_action(&pdev->dev, pcim_disable_device, pdev);

No error check?

> +	return pci_enable_device(pdev);

Maybe

	ret = pci_enable_device(...);
	if (ret)
		return ret;

	return devm_add_action_or_reset(...)?

I could think of side effects of this, so perhaps the commit message and/or
code needs a comment on why the above proposal can _not_ be used?
Philipp Stanner Jan. 17, 2024, 1:49 p.m. UTC | #2
On Tue, 2024-01-16 at 23:40 +0200, andy.shevchenko@gmail.com wrote:
> Mon, Jan 15, 2024 at 03:46:20PM +0100, Philipp Stanner kirjoitti:
> > Thanks to preceding cleanup steps, pcim_release() is now not needed
> > anymore and can be replaced by pcim_disable_device(), which is the
> > exact
> > counterpart to pcim_enable_device().
> > This permits removing further parts of the old devres API.
> > 
> > Replace pcim_release() with pcim_disable_device().
> > Remove the now surplus get_dr() function.
> 
> ...
> 
> > +       devm_add_action(&pdev->dev, pcim_disable_device, pdev);
> 
> No error check?
> 
> > +       return pci_enable_device(pdev);
> 
> Maybe
> 
>         ret = pci_enable_device(...);
>         if (ret)
>                 return ret;
> 
>         return devm_add_action_or_reset(...)?
> 
> I could think of side effects of this, so perhaps the commit message
> and/or
> code needs a comment on why the above proposal can _not_ be used?
> 

That proposal can be used, so this was simply a bug.

P.
diff mbox series

Patch

diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 7c4edcaeb8fe..87bc62be21eb 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -464,48 +464,26 @@  int pcim_intx(struct pci_dev *pdev, int enable)
 }
 EXPORT_SYMBOL_GPL(pcim_intx);
 
-static void pcim_release(struct device *gendev, void *res)
+static void pcim_disable_device(void *pdev_raw)
 {
-	struct pci_dev *dev = to_pci_dev(gendev);
-
-	if (!dev->pinned)
-		pci_disable_device(dev);
-}
-
-static struct pci_devres *get_pci_dr(struct pci_dev *pdev)
-{
-	struct pci_devres *dr, *new_dr;
-
-	dr = devres_find(&pdev->dev, pcim_release, NULL, NULL);
-	if (dr)
-		return dr;
+	struct pci_dev *pdev = pdev_raw;
 
-	new_dr = devres_alloc(pcim_release, sizeof(*new_dr), GFP_KERNEL);
-	if (!new_dr)
-		return NULL;
-	return devres_get(&pdev->dev, new_dr, NULL, NULL);
+	if (!pdev->pinned)
+		pci_disable_device(pdev);
 }
 
 /**
  * pcim_enable_device - Managed pci_enable_device()
  * @pdev: PCI device to be initialized
  *
- * Managed pci_enable_device().
+ * Managed pci_enable_device(). Device will automatically be disabled on
+ * driver detach.
  */
 int pcim_enable_device(struct pci_dev *pdev)
 {
-	struct pci_devres *dr;
-	int rc;
-
-	dr = get_pci_dr(pdev);
-	if (unlikely(!dr))
-		return -ENOMEM;
-
-	rc = pci_enable_device(pdev);
-	if (!rc)
-		pdev->is_managed = 1;
+	devm_add_action(&pdev->dev, pcim_disable_device, pdev);
 
-	return rc;
+	return pci_enable_device(pdev);
 }
 EXPORT_SYMBOL(pcim_enable_device);