* Re: [PATCH] mtd: nand: denali_pci: add missing pci_release_regions() calls
2016-11-02 17:13 [PATCH] mtd: nand: denali_pci: add missing pci_release_regions() calls Masahiro Yamada
@ 2016-11-05 7:28 ` Marek Vasut
2016-11-06 22:55 ` Boris Brezillon
2016-11-07 13:33 ` Andy Shevchenko
2 siblings, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2016-11-05 7:28 UTC (permalink / raw)
To: Masahiro Yamada, linux-mtd
Cc: Boris Brezillon, Brian Norris, Richard Weinberger, linux-kernel,
Bjorn Helgaas, Linas Vepstas, Andy Shevchenko, Enrico Jorns,
David Woodhouse, Graham Moore
On 11/02/2016 06:13 PM, Masahiro Yamada wrote:
> The probe function calls pci_request_regions(), but I do not see
> corresponding pci_release_regions() calls.
>
> While we are here, rename the jump labels to follow the guideline
> "Choose label names which say what the goto does" suggested by
> Documentation/CodingStyle.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>
> drivers/mtd/nand/denali_pci.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/denali_pci.c b/drivers/mtd/nand/denali_pci.c
> index de31514..be8152f 100644
> --- a/drivers/mtd/nand/denali_pci.c
> +++ b/drivers/mtd/nand/denali_pci.c
> @@ -76,28 +76,31 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> denali->flash_reg = ioremap_nocache(csr_base, csr_len);
> if (!denali->flash_reg) {
> dev_err(&dev->dev, "Spectra: Unable to remap memory region\n");
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto release_regions;
> }
>
> denali->flash_mem = ioremap_nocache(mem_base, mem_len);
> if (!denali->flash_mem) {
> dev_err(&dev->dev, "Spectra: ioremap_nocache failed!");
> ret = -ENOMEM;
> - goto failed_remap_reg;
> + goto unmap_reg;
> }
>
> ret = denali_init(denali);
> if (ret)
> - goto failed_remap_mem;
> + goto unmap_mem;
>
> pci_set_drvdata(dev, denali);
>
> return 0;
>
> -failed_remap_mem:
> +unmap_mem:
> iounmap(denali->flash_mem);
> -failed_remap_reg:
> +unmap_reg:
> iounmap(denali->flash_reg);
> +release_regions:
> + pci_release_regions(dev);
> return ret;
> }
>
> @@ -109,6 +112,7 @@ static void denali_pci_remove(struct pci_dev *dev)
> denali_remove(denali);
> iounmap(denali->flash_reg);
> iounmap(denali->flash_mem);
> + pci_release_regions(dev);
> }
>
> static struct pci_driver denali_pci_driver = {
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mtd: nand: denali_pci: add missing pci_release_regions() calls
2016-11-02 17:13 [PATCH] mtd: nand: denali_pci: add missing pci_release_regions() calls Masahiro Yamada
2016-11-05 7:28 ` Marek Vasut
@ 2016-11-06 22:55 ` Boris Brezillon
2016-11-07 13:33 ` Andy Shevchenko
2 siblings, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2016-11-06 22:55 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-mtd, Enrico Jorns, Andy Shevchenko, Bjorn Helgaas,
Linas Vepstas, Brian Norris, Graham Moore, linux-kernel,
Richard Weinberger, David Woodhouse
On Thu, 3 Nov 2016 02:13:10 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> The probe function calls pci_request_regions(), but I do not see
> corresponding pci_release_regions() calls.
>
> While we are here, rename the jump labels to follow the guideline
> "Choose label names which say what the goto does" suggested by
> Documentation/CodingStyle.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Applied.
Thanks,
Boris
> ---
>
> drivers/mtd/nand/denali_pci.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/denali_pci.c b/drivers/mtd/nand/denali_pci.c
> index de31514..be8152f 100644
> --- a/drivers/mtd/nand/denali_pci.c
> +++ b/drivers/mtd/nand/denali_pci.c
> @@ -76,28 +76,31 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> denali->flash_reg = ioremap_nocache(csr_base, csr_len);
> if (!denali->flash_reg) {
> dev_err(&dev->dev, "Spectra: Unable to remap memory region\n");
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto release_regions;
> }
>
> denali->flash_mem = ioremap_nocache(mem_base, mem_len);
> if (!denali->flash_mem) {
> dev_err(&dev->dev, "Spectra: ioremap_nocache failed!");
> ret = -ENOMEM;
> - goto failed_remap_reg;
> + goto unmap_reg;
> }
>
> ret = denali_init(denali);
> if (ret)
> - goto failed_remap_mem;
> + goto unmap_mem;
>
> pci_set_drvdata(dev, denali);
>
> return 0;
>
> -failed_remap_mem:
> +unmap_mem:
> iounmap(denali->flash_mem);
> -failed_remap_reg:
> +unmap_reg:
> iounmap(denali->flash_reg);
> +release_regions:
> + pci_release_regions(dev);
> return ret;
> }
>
> @@ -109,6 +112,7 @@ static void denali_pci_remove(struct pci_dev *dev)
> denali_remove(denali);
> iounmap(denali->flash_reg);
> iounmap(denali->flash_mem);
> + pci_release_regions(dev);
> }
>
> static struct pci_driver denali_pci_driver = {
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mtd: nand: denali_pci: add missing pci_release_regions() calls
2016-11-02 17:13 [PATCH] mtd: nand: denali_pci: add missing pci_release_regions() calls Masahiro Yamada
2016-11-05 7:28 ` Marek Vasut
2016-11-06 22:55 ` Boris Brezillon
@ 2016-11-07 13:33 ` Andy Shevchenko
2016-11-07 13:52 ` Boris Brezillon
2 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2016-11-07 13:33 UTC (permalink / raw)
To: Masahiro Yamada, linux-mtd
Cc: Enrico Jorns, Bjorn Helgaas, Linas Vepstas, Boris Brezillon,
Brian Norris, Graham Moore, linux-kernel, Richard Weinberger,
David Woodhouse
On Thu, 2016-11-03 at 02:13 +0900, Masahiro Yamada wrote:
> The probe function calls pci_request_regions(), but I do not see
> corresponding pci_release_regions() calls.
>
> While we are here, rename the jump labels to follow the guideline
> "Choose label names which say what the goto does" suggested by
> Documentation/CodingStyle.
NACK!
Please, remove or revert this patch.
Everything is done in pcim_release() function.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> drivers/mtd/nand/denali_pci.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/denali_pci.c
> b/drivers/mtd/nand/denali_pci.c
> index de31514..be8152f 100644
> --- a/drivers/mtd/nand/denali_pci.c
> +++ b/drivers/mtd/nand/denali_pci.c
> @@ -76,28 +76,31 @@ static int denali_pci_probe(struct pci_dev *dev,
> const struct pci_device_id *id)
> denali->flash_reg = ioremap_nocache(csr_base, csr_len);
> if (!denali->flash_reg) {
> dev_err(&dev->dev, "Spectra: Unable to remap memory
> region\n");
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto release_regions;
> }
>
> denali->flash_mem = ioremap_nocache(mem_base, mem_len);
> if (!denali->flash_mem) {
> dev_err(&dev->dev, "Spectra: ioremap_nocache
> failed!");
> ret = -ENOMEM;
> - goto failed_remap_reg;
> + goto unmap_reg;
> }
>
> ret = denali_init(denali);
> if (ret)
> - goto failed_remap_mem;
> + goto unmap_mem;
>
> pci_set_drvdata(dev, denali);
>
> return 0;
>
> -failed_remap_mem:
> +unmap_mem:
> iounmap(denali->flash_mem);
> -failed_remap_reg:
> +unmap_reg:
> iounmap(denali->flash_reg);
> +release_regions:
> + pci_release_regions(dev);
> return ret;
> }
>
> @@ -109,6 +112,7 @@ static void denali_pci_remove(struct pci_dev *dev)
> denali_remove(denali);
> iounmap(denali->flash_reg);
> iounmap(denali->flash_mem);
> + pci_release_regions(dev);
> }
>
> static struct pci_driver denali_pci_driver = {
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mtd: nand: denali_pci: add missing pci_release_regions() calls
2016-11-07 13:33 ` Andy Shevchenko
@ 2016-11-07 13:52 ` Boris Brezillon
0 siblings, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2016-11-07 13:52 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Masahiro Yamada, linux-mtd, Enrico Jorns, Bjorn Helgaas,
Linas Vepstas, Brian Norris, Graham Moore, linux-kernel,
Richard Weinberger, David Woodhouse
On Mon, 07 Nov 2016 15:33:04 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Thu, 2016-11-03 at 02:13 +0900, Masahiro Yamada wrote:
> > The probe function calls pci_request_regions(), but I do not see
> > corresponding pci_release_regions() calls.
> >
> > While we are here, rename the jump labels to follow the guideline
> > "Choose label names which say what the goto does" suggested by
> > Documentation/CodingStyle.
>
> NACK!
>
> Please, remove or revert this patch.
> Everything is done in pcim_release() function.
Indeed. I'll drop the patch.
Thanks,
Boris
>
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> > drivers/mtd/nand/denali_pci.c | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/denali_pci.c
> > b/drivers/mtd/nand/denali_pci.c
> > index de31514..be8152f 100644
> > --- a/drivers/mtd/nand/denali_pci.c
> > +++ b/drivers/mtd/nand/denali_pci.c
> > @@ -76,28 +76,31 @@ static int denali_pci_probe(struct pci_dev *dev,
> > const struct pci_device_id *id)
> > denali->flash_reg = ioremap_nocache(csr_base, csr_len);
> > if (!denali->flash_reg) {
> > dev_err(&dev->dev, "Spectra: Unable to remap memory
> > region\n");
> > - return -ENOMEM;
> > + ret = -ENOMEM;
> > + goto release_regions;
> > }
> >
> > denali->flash_mem = ioremap_nocache(mem_base, mem_len);
> > if (!denali->flash_mem) {
> > dev_err(&dev->dev, "Spectra: ioremap_nocache
> > failed!");
> > ret = -ENOMEM;
> > - goto failed_remap_reg;
> > + goto unmap_reg;
> > }
> >
> > ret = denali_init(denali);
> > if (ret)
> > - goto failed_remap_mem;
> > + goto unmap_mem;
> >
> > pci_set_drvdata(dev, denali);
> >
> > return 0;
> >
> > -failed_remap_mem:
> > +unmap_mem:
> > iounmap(denali->flash_mem);
> > -failed_remap_reg:
> > +unmap_reg:
> > iounmap(denali->flash_reg);
> > +release_regions:
> > + pci_release_regions(dev);
> > return ret;
> > }
> >
> > @@ -109,6 +112,7 @@ static void denali_pci_remove(struct pci_dev *dev)
> > denali_remove(denali);
> > iounmap(denali->flash_reg);
> > iounmap(denali->flash_mem);
> > + pci_release_regions(dev);
> > }
> >
> > static struct pci_driver denali_pci_driver = {
>
^ permalink raw reply [flat|nested] 5+ messages in thread