Discussion:
[PATCH v1 1/1] mei: me: use managed functions pcim_* and devm_*
(too old to reply)
Andy Shevchenko
2016-02-01 14:01:03 UTC
Permalink
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 <***@linux.intel.com>
---
drivers/misc/mei/hw-me.c | 4 ++--
drivers/misc/mei/pci-me.c | 40 ++++++++--------------------------------
2 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 25b1997..e26c4f4 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -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);
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 75fc9c6..3c07a02 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -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)
{
--
2.7.0
Andy Shevchenko
2016-06-05 14:48:37 UTC
Permalink
Post by 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?
Post by Andy Shevchenko
---
 drivers/misc/mei/hw-me.c  |  4 ++--
 drivers/misc/mei/pci-me.c | 40 ++++++++----------------------------
----
 2 files changed, 10 insertions(+), 34 deletions(-)
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 25b1997..e26c4f4 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -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);
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 75fc9c6..3c07a02 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -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)) {
  mei_cancel_work(dev);
  mei_disable_interrupts(dev);
  free_irq(pdev->irq, dev);
- pci_disable_msi(pdev);
- pci_iounmap(pdev, hw->mem_addr);
- kfree(dev);
- pci_release_regions(pdev);
- pci_disable_device(pdev);
  dev_err(&pdev->dev, "initialization failed.\n");
  return err;
 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)
 {
--
Andy Shevchenko <***@linux.intel.com>
Intel Finland Oy
Andy Shevchenko
2016-10-14 13:28:15 UTC
Permalink
Post by Andy Shevchenko
Post by 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.
Post by Andy Shevchenko
Post by Andy Shevchenko
---
 drivers/misc/mei/hw-me.c  |  4 ++--
 drivers/misc/mei/pci-me.c | 40 ++++++++----------------------------
----
 2 files changed, 10 insertions(+), 34 deletions(-)
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 25b1997..e26c4f4 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -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);
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 75fc9c6..3c07a02 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -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)) {
  mei_cancel_work(dev);
  mei_disable_interrupts(dev);
  free_irq(pdev->irq, dev);
- pci_disable_msi(pdev);
- pci_iounmap(pdev, hw->mem_addr);
- kfree(dev);
- pci_release_regions(pdev);
- pci_disable_device(pdev);
  dev_err(&pdev->dev, "initialization failed.\n");
  return err;
 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)
 {
--
Andy Shevchenko <***@linux.intel.com>
Intel Finland Oy
Loading...