Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Artem Bityutskiy]

On Tue, 2012-09-04 at 11:44 +0200, Julia Lawall wrote:
> > I've been bitten by the same issue recently, also cause by one of these
> > cocci devm patches. devm_clk_get is only available if the generic
> > clk_get/clk_put implementation is used. Not all architectures do this and
> > some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
> > is merely a wrapper around clk_get/clk_put there is no reason why it should
> > depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
> > available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
> > but it is on a different machine right now, will try to submit it later 
> > today.
> 
> Sorry about this.  I wasn't aware that devm_clk_get wasn't supported by
> all architectures, and I have no way of compiling code for these
> architectures...  But I wonder why it is not, since devm-ness doesn't seem
> to have anything to do with architecture-specific details?  It would be
> really nice to have it for all architectures, because the clock functions
> are just as (or at least almost as) common as kzalloc, ioremap, etc.

It looks like Lars is going to fix this.

I am personally fine if you send patches without build-testing them.
Your patches are generally of good quality and you send many of them, so
build-testing each would be too much for you. And at least for MTD, I
can build-test myself.


-- 
Best Regards,
Artem Bityutskiy


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Julia Lawall]

On Tue, 4 Sep 2012, Lars-Peter Clausen wrote:

> On 09/04/2012 10:42 AM, Artem Bityutskiy wrote:
> > Aiaiai! :-) [1] [2]
> >
> > I've build-tested this using aiaiai and it reports that this change breaks 
> > the build:
> >
> > dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc < 
> > ~/tmp/julia2.mbox
> > Tested the patch(es) on top of the following commits:
> > ba64756 Quick fixes - applied by aiaiai
> > 651c6fa JFFS2: don't fail on bitflips in OOB
> > e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
> > ea9d312 mtd: cmdlinepart: minor cleanups
> >
> > 
> > Failed to build the following commit for configuration 
> > "powerpc-mpc512x_defconfig" (architecture powerpc)":
> >
> > 0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
> >
> > ...
> > drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
> > drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set 
> > but not used [-Wunused-but-set-variable]
> > drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set 
> > but not used [-Wunused-but-set-variable]
> > drivers/built-in.o: In function `mpc5121_nfc_probe':
> > mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
> > make[1]: *** [vmlinux] Error 1
> >
> > 
> >
> > I do not really know why, but it seems that clock framework is not 
> > supported for powerpc. CCing the PPC mailing list. Preserved the context 
> > below for the PPC people.
> >
>
> I've been bitten by the same issue recently, also cause by one of these
> cocci devm patches. devm_clk_get is only available if the generic
> clk_get/clk_put implementation is used. Not all architectures do this and
> some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
> is merely a wrapper around clk_get/clk_put there is no reason why it should
> depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
> available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
> but it is on a different machine right now, will try to submit it later today.

Sorry about this.  I wasn't aware that devm_clk_get wasn't supported by
all architectures, and I have no way of compiling code for these
architectures...  But I wonder why it is not, since devm-ness doesn't seem
to have anything to do with architecture-specific details?  It would be
really nice to have it for all architectures, because the clock functions
are just as (or at least almost as) common as kzalloc, ioremap, etc.

thanks,
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Lars-Peter Clausen]

On 09/04/2012 10:42 AM, Artem Bityutskiy wrote:
> Aiaiai! :-) [1] [2]
> 
> I've build-tested this using aiaiai and it reports that this change breaks 
> the build:
> 
> dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc < 
> ~/tmp/julia2.mbox 
> Tested the patch(es) on top of the following commits:
> ba64756 Quick fixes - applied by aiaiai
> 651c6fa JFFS2: don't fail on bitflips in OOB
> e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
> ea9d312 mtd: cmdlinepart: minor cleanups
> 
> 
> Failed to build the following commit for configuration 
> "powerpc-mpc512x_defconfig" (architecture powerpc)":
> 
> 0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
> 
> ...
> drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
> drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set but 
> not used [-Wunused-but-set-variable]
> drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set but 
> not used [-Wunused-but-set-variable]
> drivers/built-in.o: In function `mpc5121_nfc_probe':
> mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
> make[1]: *** [vmlinux] Error 1
> 
> 
> 
> I do not really know why, but it seems that clock framework is not supported 
> for powerpc. CCing the PPC mailing list. Preserved the context below for the 
> PPC people.
> 

I've been bitten by the same issue recently, also cause by one of these
cocci devm patches. devm_clk_get is only available if the generic
clk_get/clk_put implementation is used. Not all architectures do this and
some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
is merely a wrapper around clk_get/clk_put there is no reason why it should
depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
but it is on a different machine right now, will try to submit it later today.

- Lars

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Artem Bityutskiy]

Aiaiai! :-) [1] [2]

I've build-tested this using aiaiai and it reports that this change breaks the 
build:

dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc < 
~/tmp/julia2.mbox 
Tested the patch(es) on top of the following commits:
ba64756 Quick fixes - applied by aiaiai
651c6fa JFFS2: don't fail on bitflips in OOB
e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
ea9d312 mtd: cmdlinepart: minor cleanups


Failed to build the following commit for configuration 
"powerpc-mpc512x_defconfig" (architecture powerpc)":

0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

...
drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set but 
not used [-Wunused-but-set-variable]
drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set but 
not used [-Wunused-but-set-variable]
drivers/built-in.o: In function `mpc5121_nfc_probe':
mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
make[1]: *** [vmlinux] Error 1



I do not really know why, but it seems that clock framework is not supported 
for powerpc. CCing the PPC mailing list. Preserved the context below for the 
PPC people.

So, not taking this patch.

References:

1. http://git.infradead.org/users/dedekind/aiaiai.git
2. http://git.infradead.org/users/dedekind/maintaining.git

On Sat, 2012-09-01 at 18:33 +0200, Julia Lawall wrote:
> From: Julia Lawall 
> 
> devm free functions should not have to be explicitly used.
> 
> The only thing left that is useful in the function mpc5121_nfc_free is the
> call to clk_disable, which is moved to the call sites.
> 
> This function also incorrectly called iounmap on devm_ioremap allocated
> data.
> 
> Use devm_request_and_ioremap in place of devm_request_mem_region and
> devm_ioremap.
> 
> Use devm_clk_get.
> 
> A semantic match that finds the first problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // 
> @@
> @@
> 
> (
> * devm_kfree(...);
> |
> * devm_free_irq(...);
> |
> * devm_iounmap(...);
> |
> * devm_release_region(...);
> |
> * devm_release_mem_region(...);
> )
> // 
> 
> Signed-off-by: Julia Lawall 
> 
> ---
>  drivers/mtd/nand/mpc5121_nfc.c |   35 +--
>  1 file changed, 5 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
> index c259c24..45183ba 100644
> --- a/drivers/mtd/nand/mpc5121_nfc.c
> +++ b/drivers/mtd/nand/mpc5121_nfc.c
> @@ -632,21 +632,6 @@ out:
>   return ret;
>  }
>  
> -/* Free driver resources */
> -static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
> -{
> - struct nand_chip *chip = mtd->priv;
> - struct mpc5121_nfc_prv *prv = chip->priv;
> -
> - if (prv->clk) {
> - clk_disable(prv->clk);
> - clk_put(prv->clk);
> - }
> -
> - if (prv->csreg)
> - iounmap(prv->csreg);
> -}
> -
>  static int __devinit mpc5121_nfc_probe(struct platform_device *op)
>  {
>   struct device_node *rootnode, *dn = op->dev.of_node;
> @@ -713,12 +698,7 @@ static int __devinit mpc5121_nfc_probe(struct 
> platform_device *op)
>   regs_paddr = res.start;
>   regs_size = resource_size();
>  
> - if (!devm_request_mem_region(dev, regs_paddr, regs_size, DRV_NAME)) {
> - dev_err(dev, "Error requesting memory region!\n");
> - return -EBUSY;
> - }
> -
> - prv->regs = devm_ioremap(dev, regs_paddr, regs_size);
> + prv->regs = devm_request_and_ioremap(dev, );
>   if (!prv->regs) {
>   dev_err(dev, "Error mapping memory region!\n");
>   return -ENOMEM;
> @@ -752,11 +732,10 @@ static int __devinit mpc5121_nfc_probe(struct 
> platform_device *op)
>   of_node_put(rootnode);
>  
>   /* Enable NFC clock */
> - prv->clk = clk_get(dev, "nfc_clk");
> + prv->clk = devm_clk_get(dev, "nfc_clk");
>   if (IS_ERR(prv->clk)) {
>   dev_err(dev, "Unable to acquire NFC clock!\n");
> - retval = PTR_ERR(prv->clk);
> - goto error;
> + return PTR_ERR(prv->clk);
>   }
>  
>   clk_enable(prv->clk);
> @@ -803,7 +782,6 @@ static int __devinit mpc5121_nfc_probe(struct 
> platform_device *op)
>   /* Detect NAND chips */
>   if (nand_scan(mtd, be32_to_cpup(chips_no))) {
>   dev_err(dev, "NAND Flash not found !\n");
> - devm_free_irq(dev, prv->irq, mtd);
>   retval = -ENXIO;
>   goto error;
>   }
> @@ -828,7 +806,6 @@ static int __devinit mpc5121_nfc_probe(struct 
> platform_device *op)
>  
>   default:
>   dev_err(dev, "Unsupported NAND flash!\n");
> - devm_free_irq(dev, prv->irq, mtd);
>   retval = -ENXIO;
>   goto 

Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Artem Bityutskiy]

Aiaiai! :-) [1] [2]

I've build-tested this using aiaiai and it reports that this change breaks the 
build:

dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc  
~/tmp/julia2.mbox 
Tested the patch(es) on top of the following commits:
ba64756 Quick fixes - applied by aiaiai
651c6fa JFFS2: don't fail on bitflips in OOB
e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
ea9d312 mtd: cmdlinepart: minor cleanups


Failed to build the following commit for configuration 
powerpc-mpc512x_defconfig (architecture powerpc):

0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

...
drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set but 
not used [-Wunused-but-set-variable]
drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set but 
not used [-Wunused-but-set-variable]
drivers/built-in.o: In function `mpc5121_nfc_probe':
mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
make[1]: *** [vmlinux] Error 1



I do not really know why, but it seems that clock framework is not supported 
for powerpc. CCing the PPC mailing list. Preserved the context below for the 
PPC people.

So, not taking this patch.

References:

1. http://git.infradead.org/users/dedekind/aiaiai.git
2. http://git.infradead.org/users/dedekind/maintaining.git

On Sat, 2012-09-01 at 18:33 +0200, Julia Lawall wrote:
 From: Julia Lawall julia.law...@lip6.fr
 
 devm free functions should not have to be explicitly used.
 
 The only thing left that is useful in the function mpc5121_nfc_free is the
 call to clk_disable, which is moved to the call sites.
 
 This function also incorrectly called iounmap on devm_ioremap allocated
 data.
 
 Use devm_request_and_ioremap in place of devm_request_mem_region and
 devm_ioremap.
 
 Use devm_clk_get.
 
 A semantic match that finds the first problem is as follows:
 (http://coccinelle.lip6.fr/)
 
 // smpl
 @@
 @@
 
 (
 * devm_kfree(...);
 |
 * devm_free_irq(...);
 |
 * devm_iounmap(...);
 |
 * devm_release_region(...);
 |
 * devm_release_mem_region(...);
 )
 // /smpl
 
 Signed-off-by: Julia Lawall julia.law...@lip6.fr
 
 ---
  drivers/mtd/nand/mpc5121_nfc.c |   35 +--
  1 file changed, 5 insertions(+), 30 deletions(-)
 
 diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
 index c259c24..45183ba 100644
 --- a/drivers/mtd/nand/mpc5121_nfc.c
 +++ b/drivers/mtd/nand/mpc5121_nfc.c
 @@ -632,21 +632,6 @@ out:
   return ret;
  }
  
 -/* Free driver resources */
 -static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
 -{
 - struct nand_chip *chip = mtd-priv;
 - struct mpc5121_nfc_prv *prv = chip-priv;
 -
 - if (prv-clk) {
 - clk_disable(prv-clk);
 - clk_put(prv-clk);
 - }
 -
 - if (prv-csreg)
 - iounmap(prv-csreg);
 -}
 -
  static int __devinit mpc5121_nfc_probe(struct platform_device *op)
  {
   struct device_node *rootnode, *dn = op-dev.of_node;
 @@ -713,12 +698,7 @@ static int __devinit mpc5121_nfc_probe(struct 
 platform_device *op)
   regs_paddr = res.start;
   regs_size = resource_size(res);
  
 - if (!devm_request_mem_region(dev, regs_paddr, regs_size, DRV_NAME)) {
 - dev_err(dev, Error requesting memory region!\n);
 - return -EBUSY;
 - }
 -
 - prv-regs = devm_ioremap(dev, regs_paddr, regs_size);
 + prv-regs = devm_request_and_ioremap(dev, res);
   if (!prv-regs) {
   dev_err(dev, Error mapping memory region!\n);
   return -ENOMEM;
 @@ -752,11 +732,10 @@ static int __devinit mpc5121_nfc_probe(struct 
 platform_device *op)
   of_node_put(rootnode);
  
   /* Enable NFC clock */
 - prv-clk = clk_get(dev, nfc_clk);
 + prv-clk = devm_clk_get(dev, nfc_clk);
   if (IS_ERR(prv-clk)) {
   dev_err(dev, Unable to acquire NFC clock!\n);
 - retval = PTR_ERR(prv-clk);
 - goto error;
 + return PTR_ERR(prv-clk);
   }
  
   clk_enable(prv-clk);
 @@ -803,7 +782,6 @@ static int __devinit mpc5121_nfc_probe(struct 
 platform_device *op)
   /* Detect NAND chips */
   if (nand_scan(mtd, be32_to_cpup(chips_no))) {
   dev_err(dev, NAND Flash not found !\n);
 - devm_free_irq(dev, prv-irq, mtd);
   retval = -ENXIO;
   goto error;
   }
 @@ -828,7 +806,6 @@ static int __devinit mpc5121_nfc_probe(struct 
 platform_device *op)
  
   default:
   dev_err(dev, Unsupported NAND flash!\n);
 - devm_free_irq(dev, prv-irq, mtd);
   retval = -ENXIO;
   goto error;
   }
 @@ -839,13 +816,12 @@ static int __devinit mpc5121_nfc_probe(struct 
 

Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Lars-Peter Clausen]

On 09/04/2012 10:42 AM, Artem Bityutskiy wrote:
 Aiaiai! :-) [1] [2]
 
 I've build-tested this using aiaiai and it reports that this change breaks 
 the build:
 
 dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc  
 ~/tmp/julia2.mbox 
 Tested the patch(es) on top of the following commits:
 ba64756 Quick fixes - applied by aiaiai
 651c6fa JFFS2: don't fail on bitflips in OOB
 e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
 ea9d312 mtd: cmdlinepart: minor cleanups
 
 
 Failed to build the following commit for configuration 
 powerpc-mpc512x_defconfig (architecture powerpc):
 
 0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
 
 ...
 drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
 drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set but 
 not used [-Wunused-but-set-variable]
 drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set but 
 not used [-Wunused-but-set-variable]
 drivers/built-in.o: In function `mpc5121_nfc_probe':
 mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
 make[1]: *** [vmlinux] Error 1
 
 
 
 I do not really know why, but it seems that clock framework is not supported 
 for powerpc. CCing the PPC mailing list. Preserved the context below for the 
 PPC people.
 

I've been bitten by the same issue recently, also cause by one of these
cocci devm patches. devm_clk_get is only available if the generic
clk_get/clk_put implementation is used. Not all architectures do this and
some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
is merely a wrapper around clk_get/clk_put there is no reason why it should
depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
but it is on a different machine right now, will try to submit it later today.

- Lars

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Julia Lawall]

On Tue, 4 Sep 2012, Lars-Peter Clausen wrote:

 On 09/04/2012 10:42 AM, Artem Bityutskiy wrote:
  Aiaiai! :-) [1] [2]
 
  I've build-tested this using aiaiai and it reports that this change breaks 
  the build:
 
  dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc  
  ~/tmp/julia2.mbox
  Tested the patch(es) on top of the following commits:
  ba64756 Quick fixes - applied by aiaiai
  651c6fa JFFS2: don't fail on bitflips in OOB
  e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
  ea9d312 mtd: cmdlinepart: minor cleanups
 
  
  Failed to build the following commit for configuration 
  powerpc-mpc512x_defconfig (architecture powerpc):
 
  0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
 
  ...
  drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
  drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set 
  but not used [-Wunused-but-set-variable]
  drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set 
  but not used [-Wunused-but-set-variable]
  drivers/built-in.o: In function `mpc5121_nfc_probe':
  mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
  make[1]: *** [vmlinux] Error 1
 
  
 
  I do not really know why, but it seems that clock framework is not 
  supported for powerpc. CCing the PPC mailing list. Preserved the context 
  below for the PPC people.
 

 I've been bitten by the same issue recently, also cause by one of these
 cocci devm patches. devm_clk_get is only available if the generic
 clk_get/clk_put implementation is used. Not all architectures do this and
 some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
 is merely a wrapper around clk_get/clk_put there is no reason why it should
 depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
 available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
 but it is on a different machine right now, will try to submit it later today.

Sorry about this.  I wasn't aware that devm_clk_get wasn't supported by
all architectures, and I have no way of compiling code for these
architectures...  But I wonder why it is not, since devm-ness doesn't seem
to have anything to do with architecture-specific details?  It would be
really nice to have it for all architectures, because the clock functions
are just as (or at least almost as) common as kzalloc, ioremap, etc.

thanks,
julia
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Artem Bityutskiy]

On Tue, 2012-09-04 at 11:44 +0200, Julia Lawall wrote:
  I've been bitten by the same issue recently, also cause by one of these
  cocci devm patches. devm_clk_get is only available if the generic
  clk_get/clk_put implementation is used. Not all architectures do this and
  some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
  is merely a wrapper around clk_get/clk_put there is no reason why it should
  depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
  available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
  but it is on a different machine right now, will try to submit it later 
  today.
 
 Sorry about this.  I wasn't aware that devm_clk_get wasn't supported by
 all architectures, and I have no way of compiling code for these
 architectures...  But I wonder why it is not, since devm-ness doesn't seem
 to have anything to do with architecture-specific details?  It would be
 really nice to have it for all architectures, because the clock functions
 are just as (or at least almost as) common as kzalloc, ioremap, etc.

It looks like Lars is going to fix this.

I am personally fine if you send patches without build-testing them.
Your patches are generally of good quality and you send many of them, so
build-testing each would be too much for you. And at least for MTD, I
can build-test myself.


-- 
Best Regards,
Artem Bityutskiy


signature.asc
Description: This is a digitally signed message part

[PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Julia Lawall]

From: Julia Lawall 

devm free functions should not have to be explicitly used.

The only thing left that is useful in the function mpc5121_nfc_free is the
call to clk_disable, which is moved to the call sites.

This function also incorrectly called iounmap on devm_ioremap allocated
data.

Use devm_request_and_ioremap in place of devm_request_mem_region and
devm_ioremap.

Use devm_clk_get.

A semantic match that finds the first problem is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// 

Signed-off-by: Julia Lawall 

---
 drivers/mtd/nand/mpc5121_nfc.c |   35 +--
 1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
index c259c24..45183ba 100644
--- a/drivers/mtd/nand/mpc5121_nfc.c
+++ b/drivers/mtd/nand/mpc5121_nfc.c
@@ -632,21 +632,6 @@ out:
return ret;
 }
 
-/* Free driver resources */
-static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
-{
-   struct nand_chip *chip = mtd->priv;
-   struct mpc5121_nfc_prv *prv = chip->priv;
-
-   if (prv->clk) {
-   clk_disable(prv->clk);
-   clk_put(prv->clk);
-   }
-
-   if (prv->csreg)
-   iounmap(prv->csreg);
-}
-
 static int __devinit mpc5121_nfc_probe(struct platform_device *op)
 {
struct device_node *rootnode, *dn = op->dev.of_node;
@@ -713,12 +698,7 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
regs_paddr = res.start;
regs_size = resource_size();
 
-   if (!devm_request_mem_region(dev, regs_paddr, regs_size, DRV_NAME)) {
-   dev_err(dev, "Error requesting memory region!\n");
-   return -EBUSY;
-   }
-
-   prv->regs = devm_ioremap(dev, regs_paddr, regs_size);
+   prv->regs = devm_request_and_ioremap(dev, );
if (!prv->regs) {
dev_err(dev, "Error mapping memory region!\n");
return -ENOMEM;
@@ -752,11 +732,10 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
of_node_put(rootnode);
 
/* Enable NFC clock */
-   prv->clk = clk_get(dev, "nfc_clk");
+   prv->clk = devm_clk_get(dev, "nfc_clk");
if (IS_ERR(prv->clk)) {
dev_err(dev, "Unable to acquire NFC clock!\n");
-   retval = PTR_ERR(prv->clk);
-   goto error;
+   return PTR_ERR(prv->clk);
}
 
clk_enable(prv->clk);
@@ -803,7 +782,6 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
/* Detect NAND chips */
if (nand_scan(mtd, be32_to_cpup(chips_no))) {
dev_err(dev, "NAND Flash not found !\n");
-   devm_free_irq(dev, prv->irq, mtd);
retval = -ENXIO;
goto error;
}
@@ -828,7 +806,6 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
 
default:
dev_err(dev, "Unsupported NAND flash!\n");
-   devm_free_irq(dev, prv->irq, mtd);
retval = -ENXIO;
goto error;
}
@@ -839,13 +816,12 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
retval = mtd_device_parse_register(mtd, NULL, , NULL, 0);
if (retval) {
dev_err(dev, "Error adding MTD device!\n");
-   devm_free_irq(dev, prv->irq, mtd);
goto error;
}
 
return 0;
 error:
-   mpc5121_nfc_free(dev, mtd);
+   clk_disable(prv->clk);
return retval;
 }
 
@@ -857,8 +833,7 @@ static int __devexit mpc5121_nfc_remove(struct 
platform_device *op)
struct mpc5121_nfc_prv *prv = chip->priv;
 
nand_release(mtd);
-   devm_free_irq(dev, prv->irq, mtd);
-   mpc5121_nfc_free(dev, mtd);
+   clk_disable(prv->clk);
 
return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

devm free functions should not have to be explicitly used.

The only thing left that is useful in the function mpc5121_nfc_free is the
call to clk_disable, which is moved to the call sites.

This function also incorrectly called iounmap on devm_ioremap allocated
data.

Use devm_request_and_ioremap in place of devm_request_mem_region and
devm_ioremap.

Use devm_clk_get.

A semantic match that finds the first problem is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 drivers/mtd/nand/mpc5121_nfc.c |   35 +--
 1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
index c259c24..45183ba 100644
--- a/drivers/mtd/nand/mpc5121_nfc.c
+++ b/drivers/mtd/nand/mpc5121_nfc.c
@@ -632,21 +632,6 @@ out:
return ret;
 }
 
-/* Free driver resources */
-static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
-{
-   struct nand_chip *chip = mtd-priv;
-   struct mpc5121_nfc_prv *prv = chip-priv;
-
-   if (prv-clk) {
-   clk_disable(prv-clk);
-   clk_put(prv-clk);
-   }
-
-   if (prv-csreg)
-   iounmap(prv-csreg);
-}
-
 static int __devinit mpc5121_nfc_probe(struct platform_device *op)
 {
struct device_node *rootnode, *dn = op-dev.of_node;
@@ -713,12 +698,7 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
regs_paddr = res.start;
regs_size = resource_size(res);
 
-   if (!devm_request_mem_region(dev, regs_paddr, regs_size, DRV_NAME)) {
-   dev_err(dev, Error requesting memory region!\n);
-   return -EBUSY;
-   }
-
-   prv-regs = devm_ioremap(dev, regs_paddr, regs_size);
+   prv-regs = devm_request_and_ioremap(dev, res);
if (!prv-regs) {
dev_err(dev, Error mapping memory region!\n);
return -ENOMEM;
@@ -752,11 +732,10 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
of_node_put(rootnode);
 
/* Enable NFC clock */
-   prv-clk = clk_get(dev, nfc_clk);
+   prv-clk = devm_clk_get(dev, nfc_clk);
if (IS_ERR(prv-clk)) {
dev_err(dev, Unable to acquire NFC clock!\n);
-   retval = PTR_ERR(prv-clk);
-   goto error;
+   return PTR_ERR(prv-clk);
}
 
clk_enable(prv-clk);
@@ -803,7 +782,6 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
/* Detect NAND chips */
if (nand_scan(mtd, be32_to_cpup(chips_no))) {
dev_err(dev, NAND Flash not found !\n);
-   devm_free_irq(dev, prv-irq, mtd);
retval = -ENXIO;
goto error;
}
@@ -828,7 +806,6 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
 
default:
dev_err(dev, Unsupported NAND flash!\n);
-   devm_free_irq(dev, prv-irq, mtd);
retval = -ENXIO;
goto error;
}
@@ -839,13 +816,12 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
retval = mtd_device_parse_register(mtd, NULL, ppdata, NULL, 0);
if (retval) {
dev_err(dev, Error adding MTD device!\n);
-   devm_free_irq(dev, prv-irq, mtd);
goto error;
}
 
return 0;
 error:
-   mpc5121_nfc_free(dev, mtd);
+   clk_disable(prv-clk);
return retval;
 }
 
@@ -857,8 +833,7 @@ static int __devexit mpc5121_nfc_remove(struct 
platform_device *op)
struct mpc5121_nfc_prv *prv = chip-priv;
 
nand_release(mtd);
-   devm_free_irq(dev, prv-irq, mtd);
-   mpc5121_nfc_free(dev, mtd);
+   clk_disable(prv-clk);
 
return 0;
 }

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/