lkml.org 
[lkml]   [2017]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [char-misc-next] mei: simplify error handling via devres function.
    From
    Date
    On Fri, 2017-01-20 at 19:22 +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
    >

    I have some comments on it, but need to go right now.
    So, I will look to this during weekend.

    > 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>
    > ---
    >  drivers/misc/mei/hw-me.c   |  13 +++---
    >  drivers/misc/mei/hw-me.h   |   4 +-
    >  drivers/misc/mei/hw-txe.c  |   8 ++--
    >  drivers/misc/mei/hw-txe.h  |   2 +-
    >  drivers/misc/mei/pci-me.c  |  74 +++++++++---------------------
    >  drivers/misc/mei/pci-txe.c | 112 ++++++++++++++--------------------
    > -----------
    >  6 files changed, 69 insertions(+), 144 deletions(-)
    >
    > diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
    > index a05375a3338a..b376acfca640 100644
    > --- a/drivers/misc/mei/hw-me.c
    > +++ b/drivers/misc/mei/hw-me.c
    > @@ -1384,21 +1384,21 @@ const struct mei_cfg mei_me_pch8_sps_cfg = {
    >  };
    >  
    >  /**
    > - * mei_me_dev_init - allocates and initializes the mei device
    > structure
    > + * devm_mei_me_init - allocates and initializes the mei device
    > structure
    >   *
    >   * @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)
    > +struct mei_device *devm_mei_me_init(struct pci_dev *pdev,
    > +     const struct mei_cfg *cfg)
    >  {
    >   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);
    > @@ -1407,4 +1407,3 @@ struct mei_device *mei_me_dev_init(struct
    > pci_dev *pdev,
    >   hw->cfg = cfg;
    >   return dev;
    >  }
    > -
    > diff --git a/drivers/misc/mei/hw-me.h b/drivers/misc/mei/hw-me.h
    > index cf64847a35b9..2c80ec46b593 100644
    > --- a/drivers/misc/mei/hw-me.h
    > +++ b/drivers/misc/mei/hw-me.h
    > @@ -70,8 +70,8 @@ extern const struct mei_cfg mei_me_pch_cpt_pbg_cfg;
    >  extern const struct mei_cfg mei_me_pch8_cfg;
    >  extern const struct mei_cfg mei_me_pch8_sps_cfg;
    >  
    > -struct mei_device *mei_me_dev_init(struct pci_dev *pdev,
    > -    const struct mei_cfg *cfg);
    > +struct mei_device *devm_mei_me_init(struct pci_dev *pdev,
    > +     const struct mei_cfg *cfg);
    >  
    >  int mei_me_pg_enter_sync(struct mei_device *dev);
    >  int mei_me_pg_exit_sync(struct mei_device *dev);
    > diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c
    > index e9f8c0aeec13..fb096c32817a 100644
    > --- a/drivers/misc/mei/hw-txe.c
    > +++ b/drivers/misc/mei/hw-txe.c
    > @@ -1196,19 +1196,19 @@ static const struct mei_hw_ops mei_txe_hw_ops
    > = {
    >  };
    >  
    >  /**
    > - * mei_txe_dev_init - allocates and initializes txe hardware specific
    > structure
    > + * devm_mei_txe_init - allocates and initializes txe hardware
    > specific structure
    >   *
    >   * @pdev: pci device
    >   *
    >   * Return: struct mei_device * on success or NULL
    >   */
    > -struct mei_device *mei_txe_dev_init(struct pci_dev *pdev)
    > +struct mei_device *devm_mei_txe_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..7083ac255497 100644
    > --- a/drivers/misc/mei/hw-txe.h
    > +++ b/drivers/misc/mei/hw-txe.h
    > @@ -62,7 +62,7 @@ static inline struct mei_device
    > *hw_txe_to_mei(struct mei_txe_hw *hw)
    >   return container_of((void *)hw, struct mei_device, hw);
    >  }
    >  
    > -struct mei_device *mei_txe_dev_init(struct pci_dev *pdev);
    > +struct mei_device *devm_mei_txe_init(struct pci_dev *pdev);
    >  
    >  irqreturn_t mei_txe_irq_quick_handler(int irq, void *dev_id);
    >  irqreturn_t mei_txe_irq_thread_handler(int irq, void *dev_id);
    > diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
    > index f9c6ec4b98ab..3918624ca40e 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,37 +173,31 @@ 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);
    > + dev = devm_mei_me_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 */
    >   irqflags = pci_dev_msi_enabled(pdev) ? IRQF_ONESHOT :
    > IRQF_SHARED;
    >  
    > - err = request_threaded_irq(pdev->irq,
    > - mei_me_irq_quick_handler,
    > - mei_me_irq_thread_handler,
    > - irqflags, KBUILD_MODNAME, dev);
    > + err = devm_request_threaded_irq(&pdev->dev, pdev->irq,
    > + mei_me_irq_quick_handler,
    > + mei_me_irq_thread_handler,
    > + irqflags, KBUILD_MODNAME,
    > dev);
    >   if (err) {
    >   dev_err(&pdev->dev, "request_threaded_irq failure.
    > irq = %d\n",
    >          pdev->irq);
    > - goto disable_msi;
    > + goto end;
    >   }
    >  
    >   if (mei_start(dev)) {
    > @@ -241,17 +235,8 @@ static int mei_me_probe(struct pci_dev *pdev,
    > const struct pci_device_id *ent)
    >  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:
    > + pci_set_drvdata(pdev, NULL);
    >   dev_err(&pdev->dev, "initialization failed.\n");
    >   return err;
    >  }
    > @@ -267,7 +252,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 +260,19 @@ 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);
    > -
    > -
    > + pci_set_drvdata(pdev, NULL);
    >  }
    > +
    >  #ifdef CONFIG_PM_SLEEP
    >  static int mei_me_pci_suspend(struct device *device)
    >  {
    > @@ -318,7 +288,7 @@ static int mei_me_pci_suspend(struct device
    > *device)
    >  
    >   mei_disable_interrupts(dev);
    >  
    > - free_irq(pdev->irq, dev);
    > + devm_free_irq(&pdev->dev, pdev->irq, dev);
    >   pci_disable_msi(pdev);
    >  
    >   return 0;
    > @@ -340,10 +310,10 @@ static int mei_me_pci_resume(struct device
    > *device)
    >   irqflags = pci_dev_msi_enabled(pdev) ? IRQF_ONESHOT :
    > IRQF_SHARED;
    >  
    >   /* request and enable interrupt */
    > - err = request_threaded_irq(pdev->irq,
    > - mei_me_irq_quick_handler,
    > - mei_me_irq_thread_handler,
    > - irqflags, KBUILD_MODNAME, dev);
    > + err = devm_request_threaded_irq(&pdev->dev, pdev->irq,
    > + mei_me_irq_quick_handler,
    > + mei_me_irq_thread_handler,
    > + irqflags, KBUILD_MODNAME,
    > dev);
    >  
    >   if (err) {
    >   dev_err(&pdev->dev, "request_threaded_irq failed: irq
    > = %d.\n",
    > diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c
    > index 58ffd30dcc91..be1709e5c1a6 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,28 +87,18 @@ 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;
    >   }
    >   }
    >  
    >   /* allocates and initializes the mei dev structure */
    > - dev = mei_txe_dev_init(pdev);
    > + dev = devm_mei_txe_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;
    > - }
    > - }
    > -
    > + memcpy(hw->mem_addr, pcim_iomap_table(pdev), sizeof(hw-
    > >mem_addr));
    >  
    >   pci_enable_msi(pdev);
    >  
    > @@ -128,19 +107,21 @@ static int mei_txe_probe(struct pci_dev *pdev,
    > const struct pci_device_id *ent)
    >  
    >   /* request and enable interrupt  */
    >   if (pci_dev_msi_enabled(pdev))
    > - err = request_threaded_irq(pdev->irq,
    > - NULL,
    > - mei_txe_irq_thread_handler,
    > - IRQF_ONESHOT, KBUILD_MODNAME, dev);
    > + err = devm_request_threaded_irq(&pdev->dev, pdev-
    > >irq,
    > + NULL,
    > + mei_txe_irq_thread_ha
    > ndler,
    > + IRQF_ONESHOT,
    > KBUILD_MODNAME,
    > + dev);
    >   else
    > - err = request_threaded_irq(pdev->irq,
    > - mei_txe_irq_quick_handler,
    > - mei_txe_irq_thread_handler,
    > - IRQF_SHARED, KBUILD_MODNAME, dev);
    > + err = devm_request_threaded_irq(&pdev->dev, pdev-
    > >irq,
    > + mei_txe_irq_quick_han
    > dler,
    > + mei_txe_irq_thread_ha
    > ndler,
    > + IRQF_SHARED,
    > KBUILD_MODNAME,
    > + dev);
    >   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,24 +154,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:
    > + pci_set_drvdata(pdev, NULL);
    >   dev_err(&pdev->dev, "initialization failed.\n");
    >   return err;
    >  }
    > @@ -206,38 +173,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);
    > + pci_set_drvdata(pdev, NULL);
    >  }
    >  
    >  
    > @@ -256,7 +210,7 @@ static int mei_txe_pci_suspend(struct device
    > *device)
    >  
    >   mei_disable_interrupts(dev);
    >  
    > - free_irq(pdev->irq, dev);
    > + devm_free_irq(&pdev->dev, pdev->irq, dev);
    >   pci_disable_msi(pdev);
    >  
    >   return 0;
    > @@ -278,15 +232,17 @@ static int mei_txe_pci_resume(struct device
    > *device)
    >  
    >   /* request and enable interrupt */
    >   if (pci_dev_msi_enabled(pdev))
    > - err = request_threaded_irq(pdev->irq,
    > - NULL,
    > - mei_txe_irq_thread_handler,
    > - IRQF_ONESHOT, KBUILD_MODNAME, dev);
    > + err = devm_request_threaded_irq(&pdev->dev, pdev-
    > >irq,
    > + NULL,
    > + mei_txe_irq_thread_ha
    > ndler,
    > + IRQF_ONESHOT,
    > KBUILD_MODNAME,
    > + dev);
    >   else
    > - err = request_threaded_irq(pdev->irq,
    > - mei_txe_irq_quick_handler,
    > - mei_txe_irq_thread_handler,
    > - IRQF_SHARED, KBUILD_MODNAME, dev);
    > + err = devm_request_threaded_irq(&pdev->dev, pdev-
    > >irq,
    > + mei_txe_irq_quick_han
    > dler,
    > + mei_txe_irq_thread_ha
    > ndler,
    > + IRQF_SHARED,
    > KBUILD_MODNAME,
    > + dev);
    >   if (err) {
    >   dev_err(&pdev->dev, "request_threaded_irq failed: irq
    > = %d.\n",
    >   pdev->irq);

    --
    Andy Shevchenko <andriy.shevchenko@linux.intel.com>
    Intel Finland Oy

    \
     
     \ /
      Last update: 2017-01-20 17:36    [W:6.491 / U:0.692 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site