Re: [PATCH v1 1/1] mei: me: use managed functions pcim_* and devm_*

[Andy Shevchenko]

On Sun, 2016-06-05 at 17:49 +0300, Andy Shevchenko wrote:
> On Mon, 2016-02-01 at 16:00 +0200, Andy Shevchenko wrote:
> > 
> > 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.

> 
> > 
> > Signed-off-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(>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(>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(>dev, "failed to get pci
> > regions.\n");
> > -   goto disable_device;
> > +   goto end;
> >     }
> >  
> >     if (dma_set_mask(>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(>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(>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(>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(>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(>dev);
> >  
> > -   hw = to_me_hw(dev);
> > -
> > -
> >     dev_dbg(>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 

Re: [PATCH v1 1/1] mei: me: use managed functions pcim_* and devm_*

[Andy Shevchenko]

On Sun, 2016-06-05 at 17:49 +0300, Andy Shevchenko wrote:
> On Mon, 2016-02-01 at 16:00 +0200, Andy Shevchenko wrote:
> > 
> > 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.

> 
> > 
> > Signed-off-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(>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(>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(>dev, "failed to get pci
> > regions.\n");
> > -   goto disable_device;
> > +   goto end;
> >     }
> >  
> >     if (dma_set_mask(>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(>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(>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(>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(>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(>dev);
> >  
> > -   hw = to_me_hw(dev);
> > -
> > -
> >     dev_dbg(>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 
Intel Finland Oy

Re: [PATCH v1 1/1] mei: me: use managed functions pcim_* and devm_*

[Andy Shevchenko]

On Mon, 2016-02-01 at 16:00 +0200, Andy Shevchenko wrote:
> 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?

> Signed-off-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(>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(>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(>dev, "failed to get pci regions.\n");
> - goto disable_device;
> + goto end;
>   }
>  
>   if (dma_set_mask(>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(>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(>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(>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(>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(>dev);
>  
> - hw = to_me_hw(dev);
> -
> -
>   dev_dbg(>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 
Intel Finland Oy

Re: [PATCH v1 1/1] mei: me: use managed functions pcim_* and devm_*

[Andy Shevchenko]

On Mon, 2016-02-01 at 16:00 +0200, Andy Shevchenko wrote:
> 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?

> Signed-off-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(>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(>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(>dev, "failed to get pci regions.\n");
> - goto disable_device;
> + goto end;
>   }
>  
>   if (dma_set_mask(>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(>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(>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(>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(>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(>dev);
>  
> - hw = to_me_hw(dev);
> -
> -
>   dev_dbg(>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 
Intel Finland Oy