[PATCH 1/2] drivers/watchdog/s3c2410_wdt.c: Convert release_resource to release_region/release_mem_region

[Julia Lawall]

On Mon, 28 Feb 2011, Kukjin Kim wrote:

> Julia Lawall wrote:
> > 
> > Request_mem_region should be used with release_mem_region, not
> > release_resource.
> > 
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression x,E;
> > @@
> > *x = request_mem_region(...)
> > ... when != release_mem_region(x)
> >     when != x = E
> > * release_resource(x);
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia at diku.dk>
> > 
> > ---
> >  drivers/watchdog/s3c2410_wdt.c |   19 ++++++++-----------
> >  1 file changed, 8 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/watchdog/s3c2410_wdt.c
> b/drivers/watchdog/s3c2410_wdt.c
> > index 25b39bf..95ae53d 100644
> > --- a/drivers/watchdog/s3c2410_wdt.c
> > +++ b/drivers/watchdog/s3c2410_wdt.c
> > @@ -402,7 +402,6 @@ static inline void s3c2410wdt_cpufreq_deregister(void)
> > 
> >  static int __devinit s3c2410wdt_probe(struct platform_device *pdev)
> >  {
> > -	struct resource *res;
> >  	struct device *dev;
> >  	unsigned int wtcon;
> >  	int started = 0;
> > @@ -416,20 +415,19 @@ static int __devinit s3c2410wdt_probe(struct
> > platform_device *pdev)
> > 
> >  	/* get the memory region for the watchdog timer */
> > 
> > -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	if (res == NULL) {
> > +	wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (wdt_mem == NULL) {
> 
> Hmm...I think, 'res' is better for platform_get_resource().
> Do we _really_ need to change the name?...

wdt_mem is a global variable.  Is res a good name for a global variable?  
One could say wdt_mem = res, if that seems better.

julia

> >  		dev_err(dev, "no memory resource specified\n");
> >  		return -ENOENT;
> >  	}
> > 
> > -	size = resource_size(res);
> > -	wdt_mem = request_mem_region(res->start, size, pdev->name);
> > -	if (wdt_mem == NULL) {
> > +	size = resource_size(wdt_mem);
> > +	if (!request_mem_region(wdt_mem->start, size, pdev->name)) {
> 
> If we keep the name 'res', don't need to change above.
> 
> >  		dev_err(dev, "failed to get memory region\n");
> >  		return -EBUSY;
> >  	}
> > 
> > -	wdt_base = ioremap(res->start, size);
> > +	wdt_base = ioremap(wdt_mem->start, size);
> 
> Same as above.
> 
> >  	if (wdt_base == NULL) {
> >  		dev_err(dev, "failed to ioremap() region\n");
> >  		ret = -EINVAL;
> > @@ -524,8 +522,8 @@ static int __devinit s3c2410wdt_probe(struct
> > platform_device *pdev)
> >  	iounmap(wdt_base);
> > 
> >   err_req:
> > -	release_resource(wdt_mem);
> > -	kfree(wdt_mem);
> > +	release_mem_region(wdt_mem->start, size);
> 
> release_mem_region(res->start, size); ?
> 
> > +	wdt_mem = NULL;
> > 
> >  	return ret;
> >  }
> > @@ -545,8 +543,7 @@ static int __devexit s3c2410wdt_remove(struct
> > platform_device *dev)
> > 
> >  	iounmap(wdt_base);
> > 
> > -	release_resource(wdt_mem);
> > -	kfree(wdt_mem);
> > +	release_mem_region(wdt_mem->start, resource_size(wdt_mem));
> 
> release_mem_region(res->start, resource_size(res)); ?
>
> >  	wdt_mem = NULL;
> >  	return 0;
> >  }
> 
> 
> Thanks.
> 
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim at samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>