* [char-misc-next V2] mei: simplify error handling via devres function.
@ 2017-01-25 16:25 Tomas Winkler
2017-01-25 16:28 ` Andy Shevchenko
0 siblings, 1 reply; 2+ messages in thread
From: Tomas Winkler @ 2017-01-25 16:25 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Alexander Usyskin, linux-kernel, Tomas Winkler, Andy Shevchenko
Use devm_ and pcim_ functions to make error handling
simpler and code smaller and tidier.
Based on original patch by
mei: me: use managed functions pcim_* and devm_*
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
https://lkml.org/lkml/2016/2/1/339
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
V2: Thanks Andy for review, integratd changes:
1. Rename devm_mei_me_init back to mei_me_dev_init
they don't really belong to devm_ namespace.
2. Revert pcim irq reservation change.
3. Drop setting driver data to NULL on exit.
4. Use directly mmio pointer received from pcim_iomap_table
drivers/misc/mei/hw-me.c | 6 ++---
drivers/misc/mei/hw-txe.c | 4 +--
drivers/misc/mei/hw-txe.h | 2 +-
drivers/misc/mei/pci-me.c | 45 +++++++------------------------
drivers/misc/mei/pci-txe.c | 67 +++++++---------------------------------------
5 files changed, 25 insertions(+), 99 deletions(-)
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index a05375a3338a..14b8cf2d3cf0 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -1389,7 +1389,7 @@ const struct mei_cfg mei_me_pch8_sps_cfg = {
* @pdev: The pci device structure
* @cfg: per device generation config
*
- * Return: The mei_device_device pointer on success, NULL on failure.
+ * Return: The mei_device pointer on success, NULL on failure.
*/
struct mei_device *mei_me_dev_init(struct pci_dev *pdev,
const struct mei_cfg *cfg)
@@ -1397,8 +1397,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/hw-txe.c b/drivers/misc/mei/hw-txe.c
index e9f8c0aeec13..b973b9fca98b 100644
--- a/drivers/misc/mei/hw-txe.c
+++ b/drivers/misc/mei/hw-txe.c
@@ -1207,8 +1207,8 @@ struct mei_device *mei_txe_dev_init(struct pci_dev *pdev)
struct mei_device *dev;
struct mei_txe_hw *hw;
- dev = kzalloc(sizeof(struct mei_device) +
- sizeof(struct mei_txe_hw), GFP_KERNEL);
+ dev = devm_kzalloc(&pdev->dev, sizeof(struct mei_device) +
+ sizeof(struct mei_txe_hw), GFP_KERNEL);
if (!dev)
return NULL;
diff --git a/drivers/misc/mei/hw-txe.h b/drivers/misc/mei/hw-txe.h
index ce3ed0b88b0c..e1e8b66d7648 100644
--- a/drivers/misc/mei/hw-txe.h
+++ b/drivers/misc/mei/hw-txe.h
@@ -45,7 +45,7 @@
* @intr_cause: translated interrupt cause
*/
struct mei_txe_hw {
- void __iomem *mem_addr[NUM_OF_MEM_BARS];
+ void __iomem * const *mem_addr;
u32 aliveness;
u32 readiness;
u32 slots;
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index f9c6ec4b98ab..bc422a87f0d4 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -149,18 +149,18 @@ 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;
}
/* set PCI host mastering */
pci_set_master(pdev);
- /* pci request regions for mei driver */
- err = pci_request_regions(pdev, KBUILD_MODNAME);
+ /* pci request regions and mapping IO device memory for mei driver */
+ err = pcim_iomap_regions(pdev, BIT(0), 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)) ||
@@ -173,24 +173,18 @@ 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);
- if (!hw->mem_addr) {
- dev_err(&pdev->dev, "mapping I/O device memory failure.\n");
- err = -ENOMEM;
- goto free_device;
- }
+ hw->mem_addr = pcim_iomap_table(pdev)[0];
+
pci_enable_msi(pdev);
/* request and enable interrupt */
@@ -244,13 +238,6 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
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;
@@ -267,7 +254,6 @@ static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
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)
@@ -276,33 +262,20 @@ 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);
if (!pci_dev_run_wake(pdev))
mei_me_unset_pm_domain(dev);
- /* disable interrupts */
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)
{
diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c
index 58ffd30dcc91..e41afeacc668 100644
--- a/drivers/misc/mei/pci-txe.c
+++ b/drivers/misc/mei/pci-txe.c
@@ -52,17 +52,6 @@ static inline void mei_txe_set_pm_domain(struct mei_device *dev) {}
static inline void mei_txe_unset_pm_domain(struct mei_device *dev) {}
#endif /* CONFIG_PM */
-static void mei_txe_pci_iounmap(struct pci_dev *pdev, struct mei_txe_hw *hw)
-{
- int i;
-
- for (i = SEC_BAR; i < NUM_OF_MEM_BARS; i++) {
- if (hw->mem_addr[i]) {
- pci_iounmap(pdev, hw->mem_addr[i]);
- hw->mem_addr[i] = NULL;
- }
- }
-}
/**
* mei_txe_probe - Device Initialization Routine
*
@@ -75,22 +64,22 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
struct mei_device *dev;
struct mei_txe_hw *hw;
+ const int mask = BIT(SEC_BAR) | BIT(BRIDGE_BAR);
int err;
- int i;
/* 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;
}
/* set PCI host mastering */
pci_set_master(pdev);
- /* pci request regions for mei driver */
- err = pci_request_regions(pdev, KBUILD_MODNAME);
+ /* pci request regions and mapping IO device memory for mei driver */
+ err = pcim_iomap_regions(pdev, mask, KBUILD_MODNAME);
if (err) {
dev_err(&pdev->dev, "failed to get pci regions.\n");
- goto disable_device;
+ goto end;
}
err = pci_set_dma_mask(pdev, DMA_BIT_MASK(36));
@@ -98,7 +87,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
if (err) {
dev_err(&pdev->dev, "No suitable DMA available.\n");
- goto release_regions;
+ goto end;
}
}
@@ -106,20 +95,10 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
dev = mei_txe_dev_init(pdev);
if (!dev) {
err = -ENOMEM;
- goto release_regions;
+ goto end;
}
hw = to_txe_hw(dev);
-
- /* mapping IO device memory */
- for (i = SEC_BAR; i < NUM_OF_MEM_BARS; i++) {
- hw->mem_addr[i] = pci_iomap(pdev, i, 0);
- if (!hw->mem_addr[i]) {
- dev_err(&pdev->dev, "mapping I/O device memory failure.\n");
- err = -ENOMEM;
- goto free_device;
- }
- }
-
+ hw->mem_addr = pcim_iomap_table(pdev);
pci_enable_msi(pdev);
@@ -140,7 +119,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (err) {
dev_err(&pdev->dev, "mei: request_threaded_irq failure. irq = %d\n",
pdev->irq);
- goto free_device;
+ goto end;
}
if (mei_start(dev)) {
@@ -173,23 +152,10 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
stop:
mei_stop(dev);
release_irq:
-
mei_cancel_work(dev);
-
- /* disable interrupts */
mei_disable_interrupts(dev);
-
free_irq(pdev->irq, dev);
pci_disable_msi(pdev);
-
-free_device:
- mei_txe_pci_iounmap(pdev, hw);
-
- kfree(dev);
-release_regions:
- pci_release_regions(pdev);
-disable_device:
- pci_disable_device(pdev);
end:
dev_err(&pdev->dev, "initialization failed.\n");
return err;
@@ -206,38 +172,25 @@ static int mei_txe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
static void mei_txe_remove(struct pci_dev *pdev)
{
struct mei_device *dev;
- struct mei_txe_hw *hw;
dev = pci_get_drvdata(pdev);
if (!dev) {
- dev_err(&pdev->dev, "mei: dev =NULL\n");
+ dev_err(&pdev->dev, "mei: dev == NULL\n");
return;
}
pm_runtime_get_noresume(&pdev->dev);
- hw = to_txe_hw(dev);
-
mei_stop(dev);
if (!pci_dev_run_wake(pdev))
mei_txe_unset_pm_domain(dev);
- /* disable interrupts */
mei_disable_interrupts(dev);
free_irq(pdev->irq, dev);
pci_disable_msi(pdev);
- pci_set_drvdata(pdev, NULL);
-
- mei_txe_pci_iounmap(pdev, hw);
-
mei_deregister(dev);
-
- kfree(dev);
-
- pci_release_regions(pdev);
- pci_disable_device(pdev);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [char-misc-next V2] mei: simplify error handling via devres function.
2017-01-25 16:25 [char-misc-next V2] mei: simplify error handling via devres function Tomas Winkler
@ 2017-01-25 16:28 ` Andy Shevchenko
0 siblings, 0 replies; 2+ messages in thread
From: Andy Shevchenko @ 2017-01-25 16:28 UTC (permalink / raw)
To: Tomas Winkler, Greg Kroah-Hartman; +Cc: Alexander Usyskin, linux-kernel
On Wed, 2017-01-25 at 18:25 +0200, Tomas Winkler wrote:
> Use devm_ and pcim_ functions to make error handling
> simpler and code smaller and tidier.
>
> Based on original patch by
> mei: me: use managed functions pcim_* and devm_*
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> https://lkml.org/lkml/2016/2/1/339
>
Thanks for an update!
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
One nitpick below and please use my SoB instead of Cc here. And you may
remove my name above in the commit message (You might leave a link if
you want to)
> 2. Revert pcim irq reservation change.
Noticed that in my origin patch it wasn't touched.
> disable_msi:
> pci_disable_msi(pdev);
But this one in error path of ->probe() and below in ->remove() can be
removed. pcim_release() will take care of it. (request_threaded_irq()
and free_irq() must be left untouched, indeed)
In suspend/resume you would use it if you want to, though I don't know
if it makes any difference.
> free_irq(pdev->irq, dev);
> pci_disable_msi(pdev);
Ditto for the second module, pci_disable_msi() can be safely removed.
(System sleep case is a separate one and perhaps needs additional
testing, thus, up to you)
> release_irq:
> free_irq(pdev->irq, dev);
> pci_disable_msi(pdev);
> free_irq(pdev->irq, dev);
> pci_disable_msi(pdev);
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-01-25 16:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-25 16:25 [char-misc-next V2] mei: simplify error handling via devres function Tomas Winkler
2017-01-25 16:28 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox