mei: me: use managed functions pcim_* and devm_*

PATCH In review Andy Shevchenko 1 participant 3 messages Feb 1, 2016 – Oct 14, 2016

Subsystems (2)

Char and misc drivers Intel management engine (mei)

lore

Share & tools

lore

Andy Shevchenko

author

2016-02-01 14:00 UTC #0 lore raw

PATCH 1/1 mei: me: use managed functions pcim_* and devm_*

This makes the error handling much more simpler than open-coding everything and in addition makes the probe function smaller an tidier.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/misc/mei/hw-me.c  |  4 ++--
 drivers/misc/mei/pci-me.c | 40 ++++++++--------------------------------
 2 files changed, 10 insertions(+), 34 deletions(-)

drivers/misc/mei/hw-me.c +2 -2

index 25b1997..e26c4f4 100644
@@ -1336,8 +1336,8 @@ struct mei_device *mei_me_dev_init(struct pci_dev *pdev,
 	struct mei_device *dev;
 	struct mei_me_hw *hw;
 
-	dev = kzalloc(sizeof(struct mei_device) +
-			 sizeof(struct mei_me_hw), GFP_KERNEL);
+	dev = devm_kzalloc(&pdev->dev, sizeof(struct mei_device) +
+				       sizeof(struct mei_me_hw), GFP_KERNEL);
 	if (!dev)
 		return NULL;
 	hw = to_me_hw(dev);

drivers/misc/mei/pci-me.c +8 ~~-32~~

index 75fc9c6..3c07a02 100644
@@ -142,7 +142,7 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return -ENODEV;
 
 	/* enable pci dev */
-	err = pci_enable_device(pdev);
+	err = pcim_enable_device(pdev);
 	if (err) {
 		dev_err(&pdev->dev, "failed to enable pci device.\n");
 		goto end;

@@ -153,7 +153,7 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	err = pci_request_regions(pdev, KBUILD_MODNAME);
 	if (err) {
 		dev_err(&pdev->dev, "failed to get pci regions.\n");
-		goto disable_device;
+		goto end;
 	}
 
 	if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) ||

@@ -166,23 +166,22 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 	if (err) {
 		dev_err(&pdev->dev, "No usable DMA configuration, aborting\n");
-		goto release_regions;
+		goto end;
 	}
 
-
 	/* allocates and initializes the mei dev structure */
 	dev = mei_me_dev_init(pdev, cfg);
 	if (!dev) {
 		err = -ENOMEM;
-		goto release_regions;
+		goto end;
 	}
 	hw = to_me_hw(dev);
 	/* mapping  IO device memory */
-	hw->mem_addr = pci_iomap(pdev, 0, 0);
+	hw->mem_addr = pcim_iomap(pdev, 0, 0);
 	if (!hw->mem_addr) {
 		dev_err(&pdev->dev, "mapping I/O device memory failure.\n");
 		err = -ENOMEM;
-		goto free_device;
+		goto end;
 	}
 	pci_enable_msi(pdev);

@@ -196,7 +195,7 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err) {
 		dev_err(&pdev->dev, "request_threaded_irq failure. irq = %d\n",
 		       pdev->irq);
-		goto disable_msi;
+		goto end;
 	}
 
 	if (mei_start(dev)) {

@@ -235,15 +234,6 @@ release_irq:
 	mei_cancel_work(dev);
 	mei_disable_interrupts(dev);
 	free_irq(pdev->irq, dev);
-disable_msi:
-	pci_disable_msi(pdev);
-	pci_iounmap(pdev, hw->mem_addr);
-free_device:
-	kfree(dev);
-release_regions:
-	pci_release_regions(pdev);
-disable_device:
-	pci_disable_device(pdev);
 end:
 	dev_err(&pdev->dev, "initialization failed.\n");
 	return err;

@@ -260,7 +250,6 @@ end:
 static void mei_me_remove(struct pci_dev *pdev)
 {
 	struct mei_device *dev;
-	struct mei_me_hw *hw;
 
 	dev = pci_get_drvdata(pdev);
 	if (!dev)

@@ -269,9 +258,6 @@ static void mei_me_remove(struct pci_dev *pdev)
 	if (mei_pg_is_enabled(dev))
 		pm_runtime_get_noresume(&pdev->dev);
 
-	hw = to_me_hw(dev);
-
-
 	dev_dbg(&pdev->dev, "stop\n");
 	mei_stop(dev);

@@ -282,20 +268,10 @@ static void mei_me_remove(struct pci_dev *pdev)
 	mei_disable_interrupts(dev);
 
 	free_irq(pdev->irq, dev);
-	pci_disable_msi(pdev);
-
-	if (hw->mem_addr)
-		pci_iounmap(pdev, hw->mem_addr);
 
 	mei_deregister(dev);
-
-	kfree(dev);
-
-	pci_release_regions(pdev);
-	pci_disable_device(pdev);
-
-
 }
+
 #ifdef CONFIG_PM_SLEEP
 static int mei_me_pci_suspend(struct device *device)
 {

Signature · 1 line

2.7.0

Andy Shevchenko

author

2016-06-05 14:48 UTC #1 lore raw

Quoted 4 lines · Andy Shevchenko

This makes the error handling much more simpler than open-coding everything and
in addition makes the probe function smaller an tidier.

It's already one release cycle passed. What is the destiny of this change?

Quoted 147 lines

Signed-off-by Andy Shevchenko <[email protected]>
---
drivers/misc/mei/hw-me.c | 4 ++-- drivers/misc/mei/pci-me.c | 40 ++++++++----------------------------
----
 2 files changed, 10 insertions(+), 34 deletions(-)
drivers/misc/mei/hw-me.c +0 -0
index 25b1997..e26c4f4 100644
@@ -1336,8 +1336,8 @@ struct mei_device *mei_me_dev_init(struct
 pci_dev *pdev,
  	struct mei_device *dev;
  	struct mei_me_hw *hw;
drivers/misc/mei/hw-me.c +2 -2
-	dev = kzalloc(sizeof(struct mei_device) +
-			 sizeof(struct mei_me_hw), GFP_KERNEL);
+	dev = devm_kzalloc(&pdev->dev, sizeof(struct mei_device) +
+				       sizeof(struct mei_me_hw),
GFP_KERNEL);
if (!dev)
return NULL;
hw = to_me_hw(dev);
drivers/misc/mei/pci-me.c +0 -0
index 75fc9c6..3c07a02 100644
@@ -142,7 +142,7 @@ static int mei_me_probe(struct pci_dev *pdev,
 const struct pci_device_id *ent)
  		return -ENODEV;
/* enable pci dev */
drivers/misc/mei/pci-me.c +1 -1
-	err = pci_enable_device(pdev);
+	err = pcim_enable_device(pdev);
if (err) {
dev_err(&pdev->dev, "failed to enable pci device.\n");
goto end;
drivers/misc/mei/pci-me.c +1 -1
@@ -153,7 +153,7 @@ static int mei_me_probe(struct pci_dev *pdev,
 const struct pci_device_id *ent)
  	err = pci_request_regions(pdev, KBUILD_MODNAME);
  	if (err) {
  		dev_err(&pdev->dev, "failed to get pci regions.\n");
-		goto disable_device;
+		goto end;
  	}
if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) ||
drivers/misc/mei/pci-me.c +1 -1
@@ -166,23 +166,22 @@ static int mei_me_probe(struct pci_dev *pdev,
 const struct pci_device_id *ent)
  	}
  	if (err) {
  		dev_err(&pdev->dev, "No usable DMA configuration,
 aborting\n");
-		goto release_regions;
+		goto end;
  	}
-
/* allocates and initializes the mei dev structure */ dev = mei_me_dev_init(pdev, cfg);
if (!dev) {
err = -ENOMEM;
drivers/misc/mei/pci-me.c +1 -1
-		goto release_regions;
+		goto end;
}
hw = to_me_hw(dev);
/* mapping IO device memory */
drivers/misc/mei/pci-me.c +1 -1
-	hw->mem_addr = pci_iomap(pdev, 0, 0);
+	hw->mem_addr = pcim_iomap(pdev, 0, 0);
if (!hw->mem_addr) {
dev_err(&pdev->dev, "mapping I/O device memory failure.\n");
err = -ENOMEM;
drivers/misc/mei/pci-me.c +1 -1
-		goto free_device;
+		goto end;
}
pci_enable_msi(pdev);
drivers/misc/mei/pci-me.c +0 -0
@@ -196,7 +195,7 @@ static int mei_me_probe(struct pci_dev *pdev,
 const struct pci_device_id *ent)
  	if (err) {
dev_err(&pdev->dev, "request_threaded_irq failure. irq = %d\n",
pdev->irq);
drivers/misc/mei/pci-me.c +1 -1
-		goto disable_msi;
+		goto end;
}

if (mei_start(dev)) {
drivers/misc/mei/pci-me.c +0 -9
@@ -235,15 +234,6 @@ release_irq:
  	mei_cancel_work(dev);
  	mei_disable_interrupts(dev);
  	free_irq(pdev->irq, dev);
-disable_msi:
-	pci_disable_msi(pdev);
-	pci_iounmap(pdev, hw->mem_addr);
-free_device:
-	kfree(dev);
-release_regions:
-	pci_release_regions(pdev);
-disable_device:
-	pci_disable_device(pdev);
end:
dev_err(&pdev->dev, "initialization failed.\n"); return err;
drivers/misc/mei/pci-me.c +0 -1
@@ -260,7 +250,6 @@ end:
  static void mei_me_remove(struct pci_dev *pdev)
  {
  	struct mei_device *dev;
-	struct mei_me_hw *hw;
dev = pci_get_drvdata(pdev);
if (!dev)
drivers/misc/mei/pci-me.c +0 -0
@@ -269,9 +258,6 @@ static void mei_me_remove(struct pci_dev *pdev)
  	if (mei_pg_is_enabled(dev))
  		pm_runtime_get_noresume(&pdev->dev);
drivers/misc/mei/pci-me.c +0 -3
-	hw = to_me_hw(dev);
-
-
dev_dbg(&pdev->dev, "stop\n");
mei_stop(dev);
drivers/misc/mei/pci-me.c +0 -0
@@ -282,20 +268,10 @@ static void mei_me_remove(struct pci_dev *pdev)
  	mei_disable_interrupts(dev);
free_irq(pdev->irq, dev);
drivers/misc/mei/pci-me.c +0 -4
-	pci_disable_msi(pdev);
-
-	if (hw->mem_addr)
-		pci_iounmap(pdev, hw->mem_addr);
mei_deregister(dev);
drivers/misc/mei/pci-me.c +0 -7
-
-	kfree(dev);
-
-	pci_release_regions(pdev);
-	pci_disable_device(pdev);
-
-
}
+
#ifdef CONFIG_PM_SLEEP
static int mei_me_pci_suspend(struct device *device) {

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy

Andy Shevchenko

author

2016-10-14 13:28 UTC #2 lore raw

Quoted 9 lines · Andy Shevchenko

Quoted 5 lines · Andy Shevchenko
This makes the error handling much more simpler than open-coding everything and
in addition makes the probe function smaller an tidier.
It's already one release cycle passed. What is the destiny of this change?

One more ping here.

Quoted 152 lines

Quoted 151 lines
Signed-off-by Andy Shevchenko <[email protected]>
---
drivers/misc/mei/hw-me.c | 4 ++-- drivers/misc/mei/pci-me.c | 40 ++++++++----------------------------
----
 2 files changed, 10 insertions(+), 34 deletions(-)
drivers/misc/mei/hw-me.c +0 -0
index 25b1997..e26c4f4 100644
@@ -1336,8 +1336,8 @@ struct mei_device *mei_me_dev_init(struct
 pci_dev *pdev,
  	struct mei_device *dev;
  	struct mei_me_hw *hw;
drivers/misc/mei/hw-me.c +2 -2
-	dev = kzalloc(sizeof(struct mei_device) +
-			 sizeof(struct mei_me_hw), GFP_KERNEL);
+	dev = devm_kzalloc(&pdev->dev, sizeof(struct mei_device) +
+				       sizeof(struct mei_me_hw),
GFP_KERNEL);
if (!dev)
return NULL;
hw = to_me_hw(dev);
drivers/misc/mei/pci-me.c +0 -0
index 75fc9c6..3c07a02 100644
@@ -142,7 +142,7 @@ static int mei_me_probe(struct pci_dev *pdev,
 const struct pci_device_id *ent)
  		return -ENODEV;
/* enable pci dev */
drivers/misc/mei/pci-me.c +1 -1
-	err = pci_enable_device(pdev);
+	err = pcim_enable_device(pdev);
if (err) {
dev_err(&pdev->dev, "failed to enable pci device.\n");
goto end;
drivers/misc/mei/pci-me.c +0 -0
@@ -153,7 +153,7 @@ static int mei_me_probe(struct pci_dev *pdev,
 const struct pci_device_id *ent)
  	err = pci_request_regions(pdev, KBUILD_MODNAME);
  	if (err) {
dev_err(&pdev->dev, "failed to get pci regions.\n");
drivers/misc/mei/pci-me.c +1 -1
-		goto disable_device;
+		goto end;
}

if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) ||
drivers/misc/mei/pci-me.c +1 -1
@@ -166,23 +166,22 @@ static int mei_me_probe(struct pci_dev *pdev,
 const struct pci_device_id *ent)
  	}
  	if (err) {
  		dev_err(&pdev->dev, "No usable DMA configuration,
 aborting\n");
-		goto release_regions;
+		goto end;
  	}
-
/* allocates and initializes the mei dev structure */ dev = mei_me_dev_init(pdev, cfg);
if (!dev) {
err = -ENOMEM;
drivers/misc/mei/pci-me.c +1 -1
-		goto release_regions;
+		goto end;
}
hw = to_me_hw(dev);
/* mapping IO device memory */
drivers/misc/mei/pci-me.c +1 -1
-	hw->mem_addr = pci_iomap(pdev, 0, 0);
+	hw->mem_addr = pcim_iomap(pdev, 0, 0);
if (!hw->mem_addr) {
dev_err(&pdev->dev, "mapping I/O device memory failure.\n");
err = -ENOMEM;
drivers/misc/mei/pci-me.c +1 -1
-		goto free_device;
+		goto end;
}
pci_enable_msi(pdev);
drivers/misc/mei/pci-me.c +0 -0
@@ -196,7 +195,7 @@ static int mei_me_probe(struct pci_dev *pdev,
 const struct pci_device_id *ent)
  	if (err) {
dev_err(&pdev->dev, "request_threaded_irq failure. irq = %d\n",
pdev->irq);
drivers/misc/mei/pci-me.c +1 -1
-		goto disable_msi;
+		goto end;
}

if (mei_start(dev)) {
drivers/misc/mei/pci-me.c +0 -9
@@ -235,15 +234,6 @@ release_irq:
  	mei_cancel_work(dev);
  	mei_disable_interrupts(dev);
  	free_irq(pdev->irq, dev);
-disable_msi:
-	pci_disable_msi(pdev);
-	pci_iounmap(pdev, hw->mem_addr);
-free_device:
-	kfree(dev);
-release_regions:
-	pci_release_regions(pdev);
-disable_device:
-	pci_disable_device(pdev);
end:
dev_err(&pdev->dev, "initialization failed.\n"); return err;
drivers/misc/mei/pci-me.c +0 -1
@@ -260,7 +250,6 @@ end:
  static void mei_me_remove(struct pci_dev *pdev)
  {
  	struct mei_device *dev;
-	struct mei_me_hw *hw;
dev = pci_get_drvdata(pdev);
if (!dev)
drivers/misc/mei/pci-me.c +0 -0
@@ -269,9 +258,6 @@ static void mei_me_remove(struct pci_dev *pdev)
  	if (mei_pg_is_enabled(dev))
  		pm_runtime_get_noresume(&pdev->dev);
drivers/misc/mei/pci-me.c +0 -3
-	hw = to_me_hw(dev);
-
-
dev_dbg(&pdev->dev, "stop\n");
mei_stop(dev);
drivers/misc/mei/pci-me.c +0 -0
@@ -282,20 +268,10 @@ static void mei_me_remove(struct pci_dev
 *pdev)
  	mei_disable_interrupts(dev);
free_irq(pdev->irq, dev);
drivers/misc/mei/pci-me.c +0 -4
-	pci_disable_msi(pdev);
-
-	if (hw->mem_addr)
-		pci_iounmap(pdev, hw->mem_addr);
mei_deregister(dev);
drivers/misc/mei/pci-me.c +0 -7
-
-	kfree(dev);
-
-	pci_release_regions(pdev);
-	pci_disable_device(pdev);
-
-
}
+
#ifdef CONFIG_PM_SLEEP
static int mei_me_pci_suspend(struct device *device) {

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy