linux-kernel - Re: [PATCH v1 1/1] mei: me: use managed functions pcim_* and devm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1465138184.1767.80.camel@linux.intel.com>
Date:	Sun, 05 Jun 2016 17:49:44 +0300
From:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:	Tomas Winkler <tomas.winkler@...el.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/1] mei: me: use managed functions pcim_* and devm_*

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 <andriy.shevchenko@...ux.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)
>  {

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