[v1] i2c: busses: i2c-designware-pcidrv:- Unmap region obtained by pcim_iomap_regions

[v1] i2c: busses: i2c-designware-pcidrv:- Unmap region obtained by pcim_iomap_regions

[Arvind Yadav]

Release IO memory mapping, if i2c_dw_pci_probe is not successful.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index d6423cf..75e6e27 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -228,8 +228,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	}
 
 	dev = devm_kzalloc(&pdev->dev, sizeof(struct dw_i2c_dev), GFP_KERNEL);
-	if (!dev)
-		return -ENOMEM;
+	if (!dev) {
+		r = -ENOMEM;
+		goto error;
+	}
 
 	dev->clk = NULL;
 	dev->controller = controller;
@@ -241,7 +243,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	if (controller->setup) {
 		r = controller->setup(pdev, controller);
 		if (r)
-			return r;
+			goto error;
 	}
 
 	dev->functionality = controller->functionality |
@@ -270,7 +272,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 
 	r = i2c_dw_probe(dev);
 	if (r)
-		return r;
+		goto error;
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
 	pm_runtime_use_autosuspend(&pdev->dev);
@@ -278,6 +280,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	pm_runtime_allow(&pdev->dev);
 
 	return 0;
+error:
+	pcim_iounmap_regions(pdev, 1 << 0);
 }
 
 static void i2c_dw_pci_remove(struct pci_dev *pdev)
-- 
1.7.9.5

Re: [v1] i2c: busses: i2c-designware-pcidrv:- Unmap region obtained by pcim_iomap_regions

[Mika Westerberg]

On Fri, Dec 23, 2016 at 03:41:33PM +0530, Arvind Yadav wrote:
> Release IO memory mapping, if i2c_dw_pci_probe is not successful.

Point of pcim_iomap_regions() is that the regions are released
automatically. So there is no need for explicit release.

Re: [v1] i2c: busses: i2c-designware-pcidrv:- Unmap region obtained by pcim_iomap_regions

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3016 bytes --]

Hi Arvind,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.9 next-20161223]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Arvind-Yadav/i2c-busses-i2c-designware-pcidrv-Unmap-region-obtained-by-pcim_iomap_regions/20161223-185314
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: x86_64-randconfig-x007-201651 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/i2c/busses/i2c-designware-pcidrv.c: In function 'i2c_dw_pci_probe':
>> drivers/i2c/busses/i2c-designware-pcidrv.c:285:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +285 drivers/i2c/busses/i2c-designware-pcidrv.c

fe20ff5c Dirk Brandewie  2011-10-06  269  	adap->class = 0;
8eb5c87a Dustin Byford   2015-10-23  270  	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
fe20ff5c Dirk Brandewie  2011-10-06  271  	adap->nr = controller->bus_num;
089c729a Mika Westerberg 2014-02-19  272  
d80d1341 Jarkko Nikula   2015-10-12  273  	r = i2c_dw_probe(dev);
d80d1341 Jarkko Nikula   2015-10-12  274  	if (r)
5c99d5d6 Arvind Yadav    2016-12-23  275  		goto error;
fe20ff5c Dirk Brandewie  2011-10-06  276  
43452335 Mika Westerberg 2013-04-10  277  	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
43452335 Mika Westerberg 2013-04-10  278  	pm_runtime_use_autosuspend(&pdev->dev);
be58eda7 Mika Westerberg 2014-02-04  279  	pm_runtime_put_autosuspend(&pdev->dev);
18dbdda8 Dirk Brandewie  2011-10-06  280  	pm_runtime_allow(&pdev->dev);
18dbdda8 Dirk Brandewie  2011-10-06  281  
fe20ff5c Dirk Brandewie  2011-10-06  282  	return 0;
5c99d5d6 Arvind Yadav    2016-12-23  283  error:
5c99d5d6 Arvind Yadav    2016-12-23  284  	pcim_iounmap_regions(pdev, 1 << 0);
fe20ff5c Dirk Brandewie  2011-10-06 @285  }
fe20ff5c Dirk Brandewie  2011-10-06  286  
0b255e92 Bill Pemberton  2012-11-27  287  static void i2c_dw_pci_remove(struct pci_dev *pdev)
fe20ff5c Dirk Brandewie  2011-10-06  288  {
fe20ff5c Dirk Brandewie  2011-10-06  289  	struct dw_i2c_dev *dev = pci_get_drvdata(pdev);
fe20ff5c Dirk Brandewie  2011-10-06  290  
18dbdda8 Dirk Brandewie  2011-10-06  291  	i2c_dw_disable(dev);
18dbdda8 Dirk Brandewie  2011-10-06  292  	pm_runtime_forbid(&pdev->dev);
18dbdda8 Dirk Brandewie  2011-10-06  293  	pm_runtime_get_noresume(&pdev->dev);

:::::: The code at line 285 was first introduced by commit
:::::: fe20ff5c7e9ca7f5369aacc7d7ca3efeda3b90fe i2c-designware: Add support for Designware core behind PCI devices.

:::::: TO: Dirk Brandewie <dirk.brandewie@gmail.com>
:::::: CC: Ben Dooks <ben-linux@fluff.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27138 bytes --]

Re: [v1] i2c: busses: i2c-designware-pcidrv:- Unmap region obtained by pcim_iomap_regions

[Jarkko Nikula]

On 12/23/2016 12:44 PM, Mika Westerberg wrote:
> On Fri, Dec 23, 2016 at 03:41:33PM +0530, Arvind Yadav wrote:
>> Release IO memory mapping, if i2c_dw_pci_probe is not successful.
>
> Point of pcim_iomap_regions() is that the regions are released
> automatically. So there is no need for explicit release.
>
Please see commit 76cf3fc844a4 ("i2c-designware-pci: use managed 
functions pcim_* and devm_*") how it simplified error handling here.

-- 
Jarkko

Re: [v1] i2c: busses: i2c-designware-pcidrv:- Unmap region obtained by pcim_iomap_regions

[Andy Shevchenko]

On Fri, 2016-12-23 at 15:41 +0530, Arvind Yadav wrote:
> Release IO memory mapping, if i2c_dw_pci_probe is not successful.
> 

Second NAK, and seems (due to kbot message) you didn't even bother to
compile test.

What are you doing?

> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
>  drivers/i2c/busses/i2c-designware-pcidrv.c |   12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
> b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index d6423cf..75e6e27 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -228,8 +228,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
>  	}
>  
>  	dev = devm_kzalloc(&pdev->dev, sizeof(struct dw_i2c_dev),
> GFP_KERNEL);
> -	if (!dev)
> -		return -ENOMEM;
> +	if (!dev) {
> +		r = -ENOMEM;
> +		goto error;
> +	}
>  
>  	dev->clk = NULL;
>  	dev->controller = controller;
> @@ -241,7 +243,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
>  	if (controller->setup) {
>  		r = controller->setup(pdev, controller);
>  		if (r)
> -			return r;
> +			goto error;
>  	}
>  
>  	dev->functionality = controller->functionality |
> @@ -270,7 +272,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
>  
>  	r = i2c_dw_probe(dev);
>  	if (r)
> -		return r;
> +		goto error;
>  
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
>  	pm_runtime_use_autosuspend(&pdev->dev);
> @@ -278,6 +280,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
>  	pm_runtime_allow(&pdev->dev);
>  
>  	return 0;
> +error:
> +	pcim_iounmap_regions(pdev, 1 << 0);
>  }
>  
>  static void i2c_dw_pci_remove(struct pci_dev *pdev)

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